Move interval object and tests into separate files #115
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThe pull request reorganizes the documentation and updates module imports across the project. The docs now rename a section and replace the Changes
Possibly related PRs
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/undate/interval.py (1)
66-75: Formatting logic.
LeveragingBaseDateConverterfor recognized formats and raising aValueErrorwhen unsupported keeps the class flexible. The debugging print statement (print(f"converter_cls == {converter_cls}")) might be removed or converted to a proper log statement for production.- print(f"converter_cls == {converter_cls}") + # Consider removing or using logger.debug(...) to avoid unexpected console output
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
docs/undate/core.rst(1 hunks)src/undate/__init__.py(1 hunks)src/undate/converters/calendars/hebrew/converter.py(1 hunks)src/undate/converters/calendars/hebrew/transformer.py(1 hunks)src/undate/converters/calendars/hijri/converter.py(1 hunks)src/undate/converters/calendars/hijri/transformer.py(1 hunks)src/undate/converters/edtf/converter.py(1 hunks)src/undate/converters/edtf/transformer.py(1 hunks)src/undate/converters/iso8601.py(1 hunks)src/undate/interval.py(1 hunks)src/undate/undate.py(3 hunks)tests/test_converters/edtf/test_edtf_transformer.py(1 hunks)tests/test_converters/test_edtf.py(1 hunks)tests/test_converters/test_iso8601.py(1 hunks)tests/test_interval.py(1 hunks)tests/test_undate.py(1 hunks)
✅ Files skipped from review due to trivial changes (9)
- src/undate/converters/edtf/transformer.py
- src/undate/converters/calendars/hijri/transformer.py
- src/undate/converters/calendars/hebrew/transformer.py
- tests/test_converters/test_edtf.py
- src/undate/converters/calendars/hijri/converter.py
- src/undate/converters/edtf/converter.py
- src/undate/converters/iso8601.py
- src/undate/converters/calendars/hebrew/converter.py
- tests/test_converters/test_iso8601.py
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: python-unit (3.13)
- GitHub Check: python-unit (3.12)
- GitHub Check: python-unit (3.11)
- GitHub Check: python-unit (3.10)
- GitHub Check: python-unit (3.9)
- GitHub Check: python-unit (3.13)
- GitHub Check: python-unit (3.12)
- GitHub Check: python-unit (3.11)
- GitHub Check: python-unit (3.10)
- GitHub Check: python-unit (3.9)
🔇 Additional comments (36)
src/undate/__init__.py (2)
4-5: Good code organization with explicit imports.The imports are now properly structured with
Calendarimported fromundate.undateandUndateIntervalimported from its new location inundate.interval. This clearly reflects the reorganization mentioned in the PR objectives.
7-7: Appropriate__all__list update.The
__all__list is correctly updated to includeCalendarand maintainUndateInterval, ensuring both are part of the public API while reflecting the reorganization of the codebase.tests/test_converters/edtf/test_edtf_transformer.py (1)
2-3: Import statement properly simplified.The import has been correctly updated to use the public API directly from
undaterather than from the specificundate.undatemodule. This is a good practice as it follows the principle of importing from the public API and shields the tests from internal implementation details.docs/undate/core.rst (2)
4-5: Section title appropriately updated.The section title has been updated from "undates and undate intervals" to "dates, intervals, and calendar", which accurately reflects the reorganized structure of the code and includes the new
Calendarclass.
10-14: Documentation references correctly updated.The documentation has been properly updated to:
- Reference the new
Calendarclass fromundate.undate- Update the reference to
UndateIntervalto point to its new location inundate.intervalThis ensures the documentation accurately reflects the new code organization.
tests/test_undate.py (3)
5-5: Import statement appropriately updated.The import has been correctly updated to import
Undate,UndateInterval, andCalendardirectly fromundaterather than fromundate.undate. This follows best practices of importing from the public API.
417-419: Test forparsemethod retains backwards compatibility.The test for the
parsemethod correctly maintains compatibility with theUndateIntervalclass even after its relocation. This ensures that the public API functionality remains consistent despite the internal code reorganization.
454-461: Test forCalendar.get_converteris appropriately maintained.The test for retrieving calendar converters has been properly maintained, ensuring that the
Calendarfunctionality works as expected after the code reorganization.src/undate/undate.py (5)
1-1: Good use of postponed evaluation of type hints.
Usingfrom __future__ import annotationsis recommended when aiming for forward references, and allows for cleaner type annotation without string literals in Python versions prior to 3.11.
3-3: Imports used for date calculations and regex.
These imports (datetime,re, andTYPE_CHECKING) look correct based on usage throughout the file. No immediate issues.Also applies to: 5-5, 6-6
8-9: Forward reference forUndateInterval.
Conditionally importingUndateIntervalunderTYPE_CHECKINGprevents runtime overhead. This approach is consistent with recommended Python best practices for forward references.
21-21: Additional date-related imports.
ImportingONE_DAY,ONE_MONTH_MAX,Date,DatePrecision, andTimedeltafrom the same module keeps related date/time utilities grouped together and improves clarity.
225-225: Clarify the parse return type docstring vs. usage.
The method signature now correctly supports returning anUndateInterval, but the docstring warns that “some parsers may return intervals.” Ensure that any parser truly capable of producing an interval is properly documented and tested. Otherwise, you may consider limiting the default use.tests/test_interval.py (11)
1-9: Imports and fixtures.
All imports here (calendar,datetime,pytest,Undate,UndateInterval,Timedelta) appear relevant to the test suite. This is a clean setup.
10-28: Check for robust typing intest_init_types.
These tests ensure correct construction ofUndateIntervalfromdatetime.dateand other unsupported types. They comprehensively cover edge cases (e.g., integers or strings). Great approach to guaranteeing correct behavior.
29-32: Interval validation.
The test correctly checks for invalid intervals where thelatestis before theearliest. This strongly validates core constructor logic.
33-43: String representation.
The test confirms interval string formatting (e.g."2022/2023","2022/2023-05", etc.), matching the expected range representation. The coverage is straightforward and beneficial.
44-57: Format method coverage.
These checks confirm that both “EDTF” and “ISO8601” formats produce the correct string output, including open-ended intervals (../2000,2000/..). This helps ensure consistent user-facing representations.
58-66:__repr__details.
Validation of labeled vs. unlabeled interval representations is comprehensive. Good job verifying that labels show up appropriately.
68-75: Open range string tests.
Ensures coverage of partially known intervals and open-ended combos (e.g."0900/","../1900-12") to confirm consistent text output.
76-84: Equality checks.
These tests verify that intervals with matchingearliestandlatestare considered equal. Straightforward and thorough.
85-96: Inequality checks.
Covering distinct earliest and latest boundaries ensures correct inequality checks. All relevant differences are tested.
97-99: Minimum allowable year constraints.
VerifyingUndate.MIN_ALLOWABLE_YEARis not a leap year ensures consistent logic across boundary conditions.
100-146: Duration tests for known, unknown, and edge cases.
Thoroughly tests inclusive day calculations, unknown-year intervals, edge leaps (e.g. February spanning). Ensures correct returns (NotImplementedor exceptions) for open-ended or partially open intervals. This coverage is excellent.src/undate/interval.py (12)
1-5: Imports for datetime and typing.
Straightforward usage aligning with the class logic. No issues noted.
7-10:Undateand date tools.
These imports fromundatemodules are well organized. EnsureONE_YEARis used accurately for year computations.
12-12: NewUndateIntervalclass.
Introducing this class clarifies date-range logic separate fromUndate. The additional docstring helps maintain readability.
31-36: Constructor signature.
Optional earliest/latestUndateobjects and an optional label are intuitive. Nicely typed for clarity in the docstring.
37-47: Earliest date type handling.
Conversion fromdatetime.datetoUndateis a good convenience. The ValueError for unsupported types is explicit and user-friendly.
48-52: Latest date type handling.
Symmetrically done, matching earliest date logic. This consistency is beneficial to prevent silent conversion mismatches.
54-56: Interval validity check.
Raising a ValueError iflatest <= earliestensures data integrity for legitimate intervals. Good early check.
58-60: Assigning constructor parameters.
Straightforward assignment of earliest, latest, label. Clear and maintainable.
62-65:__str__method returning EDTF-like open intervals.
The..prefix usage for open start is creative and consistent with extended interval formats.
76-80:__repr__includes optional label.
Display of label in single quotes is consistent with howUndatedoes it, ensuring a uniform debugging experience.
81-84: Equality check.
Comparing earliest and latest suffices for interval equality. Straightforward implementation.
85-126: Comprehensive duration logic.
- Correctly returns
NotImplementedfor open-ended intervals.- Inclusive day count for fully known intervals.
- Intelligent handling of unknown-year intervals using one-year offsets.
- Raises
NotImplementedErrorwhen only one year is known.Overall, well-structured. This behavior is fully covered in
tests/test_interval.py.
|
@coderabbitai ignore |
✅ Actions performedReviews paused. |
Pulled the file reorganization out of #114 in hopes it would make it easier to review the new functionality
Summary by CodeRabbit
Documentation
New Features
Refactor
Tests
UndateIntervalclass, reducing test coverage.