Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BWRedits #9

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

BWRedits #9

wants to merge 18 commits into from

Conversation

rogersbw
Copy link
Collaborator

@rogersbw rogersbw commented Jan 8, 2025

This PR combines some edits on 2 fronts:

  1. Reorganizing the repo and switching to uv based virtual enviornment/dependency management.

  2. Adding in some data_processing functions with the goal of allowing for unbalanced blocks (due to missing data, multiple data sources having different data availability, etc.), and for chunking up the diffs calculation for the kernel density estimator.

Copy link
Contributor

@elray1 elray1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

98% of my comments here are really minor/picky things, basically just formatting. I had one actual question about whether we want to be doing a reduce_all on y vectors that's the same as what we do for x. x has an extra dimension for features, but y doesn't.

For the minor things, one thing to point out is that there is a tool called ruff that can be helpful for identifying and automatically fixing these things. See the information on installing and using ruff here: https://github.com/astral-sh/ruff?tab=readme-ov-file#getting-started

README.md Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
src/jacques/jacques.py Outdated Show resolved Hide resolved
src/jacques/jacques.py Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this file from this PR since there's not really anything here yet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!

tests/jacques/data_processing/test_assign_blocks.py Outdated Show resolved Hide resolved
@rogersbw rogersbw requested a review from elray1 January 16, 2025 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants