-
Notifications
You must be signed in to change notification settings - Fork 2
fix: handle end of year properly for next_rate_schedule #196
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
Conversation
📝 WalkthroughWalkthroughThis pull request streamlines Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Pylint (4.0.4)openeihttp/__init__.pyThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
openeihttp/__init__.pyopeneihttp/cache.pysetup.pytox.ini
🧰 Additional context used
🪛 Ruff (0.14.10)
openeihttp/__init__.py
269-269: Test for membership should be not in
Convert to not in
(E713)
🔇 Additional comments (5)
tox.ini (1)
43-43: LGTM! Reasonable adjustment to line length limit.Increasing the max line length from 88 to 100 characters provides more flexibility while maintaining readability.
openeihttp/cache.py (1)
1-1: LGTM! Docstring formatting improvement.The docstring formatting is now consistent with standard Python conventions.
setup.py (1)
9-9: LGTM! Appropriate version bump for a bug fix.The patch version increment from 0.2.7 to 0.2.8 correctly follows semantic versioning for a bug fix release.
openeihttp/__init__.py (2)
267-267: LGTM! Clearer, more concise docstring.The simplified docstring is more direct and easier to understand.
275-333: End-of-year and month boundary transitions are already comprehensively tested.The year-wrapping logic is correct and mathematically sound. The code already includes six test cases covering all the edge cases you mentioned:
test_get_next_rate_structure_next_year(Nov 1, 2024 → May 1, 2025) explicitly validates year boundary transitionstest_get_next_rate_structure_next_monthcovers month-end transitionstest_get_next_rate_structure_next_month_weekend_startcovers weekend/weekday transitions at month boundaries- Additional tests cover same-day, next-day, and weekend loop scenarios
The month_idx loop correctly wraps from December to January: when
month_idx=12, the calculationyear = start.year + (month_idx // 12)increments the year, andmonth = (month_idx % 12) + 1resets to month 1.No additional test coverage is needed.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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
♻️ Duplicate comments (1)
openeihttp/__init__.py (1)
269-270: Membership test fix looks good.The
not inoperator is now correctly placed, following Python conventions.
🧹 Nitpick comments (1)
openeihttp/__init__.py (1)
301-318: Rate change detection logic is sound.The logic correctly handles:
- Same-day rate changes (line 305-306)
- Next-day transitions within the same month (lines 309-318)
- Skipping Friday and Sunday for next-day returns since those cross schedule type boundaries
Consider adding a brief comment explaining why
[4, 6](Friday/Sunday) are excluded—they require schedule-type transitions handled by the outer loop.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
openeihttp/__init__.py
🔇 Additional comments (2)
openeihttp/__init__.py (2)
322-325: This is the correct fix for end-of-year handling.Changing from
>to!=ensures the month boundary check works across year boundaries. Previously, when transitioning from December (12) to January (1), the comparison1 > 12wasFalse, causing incorrect behavior. With!=, the check1 != 12correctly identifies the month change and breaks to advance through the month loop.
333-333: Appropriate fallback return value.Returning
(None, current_structure)when no rate change is found within the 12-month window is sensible—callers getNonefor the time (indicating no upcoming change) and the current rate structure as the rate value.
Related to firstof9/ha-openei#96
Summary by CodeRabbit
Refactor
Style
Chores
✏️ Tip: You can customize this high-level summary in your review settings.