-
Notifications
You must be signed in to change notification settings - Fork 18
Fix: Allow partial data generation on the last day of the month #593
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
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @esebesto, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a bug where data generation could be inadvertently skipped when the system was run on the last day of a month. The fix involves refining the logic for calculating the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request aims to fix a bug that prevented partial data generation on the last day of the month. The changes correctly identify the need to handle this edge case. However, the implementation of the condition to detect this scenario is not fully robust and could lead to incorrect behavior when the script is run on a date that shares the same day number as the last day of a different month in the dataset (e.g., running on March 31st for data from January). I've provided a suggestion to make the date comparison more precise by including year and month checks.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (50.0%) is below the target coverage (90.0%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #593 +/- ##
=======================================
- Coverage 93.5% 93.4% -0.1%
=======================================
Files 56 56
Lines 4730 4732 +2
Branches 663 664 +1
=======================================
- Hits 4423 4421 -2
- Misses 165 167 +2
- Partials 142 144 +2 🚀 New features to boost your workflow:
|
|
/gemini review |
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.
Code Review
This pull request correctly addresses a bug that prevented partial data generation on the last day of the month by ensuring the end date is not prematurely reset to midnight. The logic change in nise/__main__.py is sound. I've added a suggestion to refactor the new conditional logic for improved readability and to remove a redundant timezone replacement. The version bump is also appropriate for this fix.
feaff90 to
13617fc
Compare
13617fc to
778e690
Compare
|
/gemini review |
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.
Code Review
This pull request correctly fixes a bug that prevented partial data generation on the last day of the month by adjusting the end date logic. The change in nise/__main__.py ensures that the calculated end date from the data generators is respected, preventing a scenario where the start date becomes later than the end date. My review includes a suggestion to improve the readability of the new conditional logic.
|
/gemini review |
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.
Code Review
This pull request aims to fix a bug preventing partial data generation on the last day of the month. While the intent is correct, the implementation of the fix introduces a new logical flaw. The condition to decide whether to use a partial day's timestamp is overly complex and can lead to incorrect end dates under certain circumstances, potentially shortening the data generation period unexpectedly. I have provided a review comment with a simplified and more robust code suggestion that correctly addresses the original bug without introducing new issues.
|
/gemini review |
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.
Code Review
This pull request correctly fixes a bug that prevented partial data generation on the last day of the month. The logic change in nise/__main__.py properly handles the end_date to preserve the time component when running on the last day. I've added one suggestion to improve the readability and maintainability of the new date comparison logic. Otherwise, the changes look good.
|
/gemini review |
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.
Code Review
This pull request correctly addresses a bug that could cause data generation to be skipped when run on the last day of the month. The logic to preserve the end time on the last day is sound. I've added one suggestion to refactor the conditional logic for improved readability and to avoid redundant calls, using an assignment expression.
This PR addresses a bug where data generation could be skipped when run on the last day of the month.