-
Notifications
You must be signed in to change notification settings - Fork 1
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
added storage functionality for raw windows #121
Conversation
WalkthroughThe changes introduce new configuration options for specifying a directory and filename for storing window statistics, enhancing output handling capabilities. The main function has been updated to include logic for creating directories and writing the result DataFrame to a Parquet file when specified paths are provided. Additionally, a new test function has been implemented to validate the command-line interface for processing medication data and ensure the correct functionality of output file generation. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/aces/main.py (1 hunks)
- src/aces/configs/aces.yaml (1 hunks)
- tests/test_meds.py (1 hunks)
Additional comments not posted (5)
src/aces/configs/aces.yaml (2)
15-15
: LGTM!The comment is clear and provides necessary context for the new configuration option.
The code changes are approved.
16-16
: LGTM!The new configuration option
window_stats_filepath
is correctly added and follows the existing structure.The code changes are approved.
src/aces/__main__.py (2)
176-176
: LGTM!The conditional check for
cfg.window_stats_filepath
is correctly implemented and follows the existing structure.The code changes are approved.
177-177
: LGTM!The logic to write the
result
DataFrame to a Parquet file ifcfg.window_stats_filepath
is specified is correctly implemented and follows the existing structure.The code changes are approved.
tests/test_meds.py (1)
373-458
: LGTM!The new test function
test_meds_window_storage
is comprehensive and correctly validates the functionality of the CLI related to processing medication data. The use of a temporary directory for managing input and output files is a good practice.The code changes are approved.
…s check file directories
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (1)
tests/test_meds.py (1)
373-376
: Consider adding a docstring.Adding a docstring to the function will improve readability and maintainability by explaining the purpose and functionality of the test.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/aces/main.py (1 hunks)
- src/aces/configs/aces.yaml (1 hunks)
- tests/test_meds.py (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/aces/main.py
- src/aces/configs/aces.yaml
Additional context used
Ruff
tests/test_meds.py
409-409: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
Additional comments not posted (2)
tests/test_meds.py (2)
384-388
: LGTM!The setup of input files, task configurations, and expected outputs is correctly implemented.
389-425
: LGTM!The logic for managing the temporary directory, writing input files, and task configurations is correctly implemented. The command setup is also appropriate.
Tools
Ruff
409-409: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
…necessary .keys() from loop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- tests/test_meds.py (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/test_meds.py
Edit: oops sorry, I missed this at the top, yes it resolves #97. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- src/aces/main.py (1 hunks)
- src/aces/configs/aces.yaml (1 hunks)
- tests/test_meds.py (3 hunks)
Files skipped from review as they are similar to previous changes (3)
- src/aces/main.py
- src/aces/configs/aces.yaml
- tests/test_meds.py
Resolves issue #97
Summary by CodeRabbit
New Features
Tests