-
Notifications
You must be signed in to change notification settings - Fork 1
Release v0.4 #118
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
Release v0.4 #118
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
updated ismi sample data
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Caution Review failedThe pull request is closed. WalkthroughThis pull request refactors multiple workflows, documentation, source code, examples, and tests. The CI/CD files now use a new utility, Changes
Sequence Diagram(s)sequenceDiagram
participant GH as "GitHub Actions"
participant Repo as "Repository Code"
participant UV as "uv Utility"
GH->>Repo: Checkout repository
Repo--)GH: Provide workflow files
GH->>UV: Execute `uv sync --extra`
UV--)GH: Install dependencies & enable caching
GH->>UV: Run `uv run` commands (checks/tests)
UV--)GH: Return execution results
sequenceDiagram
participant User as "User/Developer"
participant U as "Undate Class"
participant C as "Calendar Enum"
participant Conv as "Calendar Converter"
User->>U: Call `Undate.parse(date_string, format)`
U->>C: Set calendar via `set_calendar()`
C->>Conv: Retrieve appropriate converter (Gregorian/Hebrew/Islamic)
Conv->>U: Convert date components (to Gregorian values)
U-->>User: Return enhanced Undate object with calendar info
Possibly related PRs
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
✨ Finishing Touches
🪧 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 (
|
Interval logic improvements: type/range validation, intersection method, contains/in functionality
Try using uv for unit test workflow
Support conversion from internal Date class to Undate #119
Use `islamic` for Islamic/Hijri calendar classes #120
Reorganize examples and update readme / contents #121
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 comments (1)
tests/test_interval.py (1)
214-234
: 💡 Verification agent🧩 Analysis chain
Edge case handling in containment tests.
The tests for containment cover important edge cases including open intervals. The comment "fully open interval - is this even meaningful?" raises a good question - consider documenting the expected behavior of fully open intervals in the class docstring.
🏁 Script executed:
#!/bin/bash # Checking if the UndateInterval class docstring addresses open interval behavior # Check the documentation of UndateInterval class rg -A 5 "class UndateInterval" src/undate/Length of output: 360
Docstring Clarification Needed for Fully Open Intervals
The containment tests appropriately cover edge cases, including those for open intervals. However, the
UndateInterval
class docstring insrc/undate/interval.py
currently lacks any description regarding the expected behavior of fully open intervals. Please update the docstring to explicitly document how fully open intervals should be handled.
- File to update:
src/undate/interval.py
(in theUndateInterval
class definition)
🧹 Nitpick comments (31)
src/undate/converters/calendars/islamic/parser.py (1)
1-9
: Add docstrings for better documentationWhile the code is functional, adding documentation would improve maintainability.
Consider adding a module docstring to explain the purpose of this parser:
import pathlib from lark import Lark +""" +Parser for Islamic dates based on a Lark grammar. +This module provides functionality to parse Islamic calendar dates +in various formats as defined in the associated grammar file. +""" + grammar_path = pathlib.Path(__file__).parent / "islamic.lark"src/undate/__init__.py (1)
7-7
: Consider reordering all to match import orderWhile the all list correctly includes all exported symbols, the order of UndateInterval and Calendar doesn't match the import order above. Consider reordering for consistency.
-__all__ = ["Undate", "UndateInterval", "Calendar", "DatePrecision", "__version__"] +__all__ = ["Undate", "Calendar", "UndateInterval", "DatePrecision", "__version__"]tests/test_converters/test_edtf.py (1)
66-69
: Removed test for empty Undate since it's no longer supportedThe commented-out test code reflects the architectural change that Undate objects now require at least one date component (year, month, or day) to be specified, as enforced by the ValueError check in the Undate.init method.
Consider updating the comment to more clearly explain why the test is no longer needed:
-# empty_undate = Undate() # undate with no date information no longer supported +# Test removed: empty Undate objects are no longer supported as Undate.__init__ now +# explicitly requires at least one of year, month, or day to be specifiedexamples/ismi/README.md (2)
7-14
: Fix Markdown list indentationThe Markdown list indentation is inconsistent, which might cause rendering issues. The static analysis tool flagged loose punctuation marks.
- `crm:E52_Time-Span` - - `crm:P2_has_type`: date type - - "datetype:day" - - `crm:P82_at_some_time_within`: given day (xsd:date) - - "datetype:year" - - `crm:P82a_begin_of_the_begin`: first day (xsd:date), `crm:P82b_end_of_the_end`: last day of year (xsd:date) - - "datetype:range" - - `crm:P82a_begin_of_the_begin`: first day (xsd:date), `crm:P82b_end_of_the_end`: last day of range (xsd:date) + - `crm:P2_has_type`: date type + - "datetype:day" + - `crm:P82_at_some_time_within`: given day (xsd:date) + - "datetype:year" + - `crm:P82a_begin_of_the_begin`: first day (xsd:date), `crm:P82b_end_of_the_end`: last day of year (xsd:date) + - "datetype:range" + - `crm:P82a_begin_of_the_begin`: first day (xsd:date), `crm:P82b_end_of_the_end`: last day of range (xsd:date)🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: Loose punctuation mark.
Context: ...crm:E52_Time-Span-
crm:P2_has_type`: date type - "datetype:day" - ...(UNLIKELY_OPENING_PUNCTUATION)
15-20
: Fix Markdown list indentation (continued)Same issue with indentation in this section of the list.
- - `crm:P1_is_identified_by` - - `crm:E41_Appellation` - - `rdfs:label`: textual representation of timespan (e.g. "901 Rabīʿ I 14 (islamic)") - - `crm:P2_has_type`: calendar type (calendar the date was entered in) - - "calendartype:julian", "calendartype:islamic", "calendartype:gregorian" - - `crm:P3_has_note`: textual note with additional information + - `crm:P1_is_identified_by` + - `crm:E41_Appellation` + - `rdfs:label`: textual representation of timespan (e.g. "901 Rabīʿ I 14 (islamic)") + - `crm:P2_has_type`: calendar type (calendar the date was entered in) + - "calendartype:julian", "calendartype:islamic", "calendartype:gregorian" + - `crm:P3_has_note`: textual note with additional information🧰 Tools
🪛 LanguageTool
[uncategorized] ~17-~17: Loose punctuation mark.
Context: ...crm:E41_Appellation
-rdfs:label
: textual representation of timespan (e.g...(UNLIKELY_OPENING_PUNCTUATION)
README.md (1)
184-184
: Use consistent horizontal rule styleThe horizontal rule on this line uses asterisks (
* * *
) which is inconsistent with other horizontal rules in the document that use dashes (---
).-* * * +---🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
184-184: Horizontal rule style
Expected: ---; Actual: * * *(MD035, hr-style)
src/undate/converters/iso8601.py (1)
104-107
: Minor typo in comment.There's a small typo in the comment: "genereate" should be "generate".
- # to genereate --MM-DD unknown year format + # to generate --MM-DD unknown year formatsrc/undate/interval.py (3)
7-19
: Consider clarifying docstrings for class attributes.
While the docstrings on lines 13–19 provide usage context, it could be beneficial to explicitly mention thatearliest
andlatest
can beNone
to indicate open-ended intervals. This ensures users fully understand the class API up front.
51-53
: Revisit interval validation for equal boundaries.
Currently, the checkif latest and earliest and latest <= earliest:
raises aValueError
whenlatest == earliest
. Depending on your application, an interval with identical start and end dates may be legitimate (e.g., a single-day event). Consider using<
instead of<=
if you want to allow a zero-length interval.
126-157
: Check open interval bounds in__contains__
.
Your logic correctly handles open-ended intervals (earliest=None
orlatest=None
) by skipping upper or lower bound checks. Ensure client code is aware that membership is always considered true at the open end. If partial bounding is undesired (e.g., an interval that starts at a non-defined date is always "before" a date), you might want to raise a warning or disallow open intervals altogether.src/undate/converters/calendars/hebrew/transformer.py (2)
7-7
: Fix typographical error in class docstring.
Replace “convience” with “convenience” to maintain clarity and professionalism.-"""Undate convience subclass; sets default calendar to Hebrew.""" +"""Undate convenience subclass; sets default calendar to Hebrew."""
34-41
: Consider validating month rule names inmonth
method.
Here,tree.data.split("_")[-1]
is converted directly to a month integer. If a parse rule is malformed (e.g.,month_XX
), it could break. Consider validating the split substring or adding a fallback if grammar changes occur.tests/test_converters/test_calendars/test_hebrew/test_hebrew_converter.py (3)
78-84
: Consider expanding partially unknown date tests.
While the partial unknown month ("XX"
or"1X"
) coverage is excellent, also testing uncertain years (e.g.,"XXXX"
) and borderline negative year inputs would strengthen confidence in the converter’s behavior across edge cases.
103-103
: Minor grammatical nitpick.
In the comment “# of last semantic monthin the year”, add a space after “month” to maintain readability.-# of last semantic monthin the year +# of last semantic month in the year
139-139
: Typographical spacing fix.
The comment has a misplaced space: “c ontained”. Updating it ensures consistent readability across your test documentation.-# so it falls within or is c ontained by July 1056 +# so it falls within or is contained by July 1056src/undate/converters/calendars/islamic/islamic.lark (1)
8-13
: Consider implementing the TODOs in future iterationsThe TODOs mentioning date ranges and qualifiers (like "first decade of") would enhance the library's capabilities for handling more complex date expressions common in historical texts.
src/undate/converters/calendars/islamic/converter.py (1)
66-67
: Consider implementing Gregorian to Islamic conversionThe commented question about supporting conversion in the opposite direction (Gregorian to Islamic) could be addressed in a future release to provide more complete functionality.
#!/bin/bash # Check if convertdate.islamic supports conversion from Gregorian to Islamic grep -r "from_gregorian" $(find -name "*.py" | grep convertdate)src/undate/converters/calendars/hebrew/hebrew.lark (3)
8-13
: Consider prioritizing the TODO items.The TODOs for handling date ranges and qualifiers are important features that would enhance the parser's flexibility. It would be helpful to create issues to track these enhancements.
14-15
: Add validation for historically valid years.The current implementation accepts any digit sequence for years. Consider adding validation for historically valid Hebrew calendar years to prevent parsing errors with implausible dates.
- year: /\d+/ + // Hebrew calendar starts with year 1 in 3761 BCE + // Valid years should be between 1 and ~6000 (current year ~5784) + year: /[1-9]\d{0,3}/
31-32
: Consider adding semantic validation for days per month.The current implementation accepts days 1-30 for all months, but Hebrew months have either 29 or 30 days depending on the month and year. Consider adding semantic validation post-parsing.
tests/test_interval.py (1)
106-152
: Add test for leap year duration calculation.While you test non-leap year duration (line 141-144), you should also include a test for leap year duration to ensure proper handling of February 29.
+ # Test leap year duration + def test_duration_leap_year(self): + # Test interval spanning Feb 29 in a leap year + leap_feb = UndateInterval( + Undate(2020, 2, 28), Undate(2020, 3, 1) + ).duration() + assert leap_feb.days == 3 # 28, 29, 1src/undate/converters/calendars/hebrew/converter.py (3)
13-18
: Consider refining the class docstring for clarity.The phrase "Anno Mundicalendar" could be more readable as "Anno Mundi calendar." A small wording change might help maintain clarity and consistency.
56-75
: Consider trimming whitespace for more robust parsing.Currently, the
parse
method does not handle leading/trailing whitespace in the input string. Stripping whitespace before parsing may prevent potential parsing errors or user confusion.def parse(self, value: str) -> Union[Undate, UndateInterval]: ... - if not value: + value = value.strip() + if not value: raise ValueError("Parsing empty string is not supported")
77-79
: Evaluate the need for reverse conversion.The comment suggests possibly converting from a Gregorian date back to a Hebrew date. If this is in scope, you may consider adding a
from_gregorian
method to support round-trip conversions.pyproject.toml (1)
12-12
: Confirm the necessity of installing NumPy by default.NumPy can be quite large if it’s not essential for every usage scenario. Consider making it part of an optional dependency group if it’s not strictly required for core functionality.
src/undate/converters/base.py (2)
114-115
: Validate converter name collisions.The
available_converters
method uses each converter’sname
as the dictionary key. If two subclasses share the same name, the latter will overwrite the former. Consider handling collisions or raising an error.
116-136
: Review recursive subclass discovery.The
subclasses
method recurses to find nested subclasses and excludesBaseCalendarConverter
. While this is appropriate, ensure that any newly introduced abstract base classes are also excluded if needed.tests/test_undate.py (1)
5-5
: Remove or justifyUndateInterval
import if not used.
It looks like theUndateInterval
class was removed elsewhere, but it's still imported on this line. If it's no longer referenced in the test suite (beyond the one or two parse tests, if any), consider removing it to keep the imports clean.src/undate/undate.py (3)
199-202
: Chain the new exception to preserve the original traceback.
Re-raising a new exception in anexcept KeyError:
block without referencing the original exception can sometimes obscure debugging information. Consider chaining explicitly (e.g.,raise ValueError(f"Calendar
{calendar}is not supported") from err
) to retain context.except KeyError as e: - raise ValueError(f"Calendar `{calendar}` is not supported") + raise ValueError(f"Calendar `{calendar}` is not supported") from e🧰 Tools
🪛 Ruff (0.8.2)
202-202: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
299-302
: Avoid iterating over.keys()
when checking dictionary keys.
Instead of callingself.initial_values.keys()
, you can iterate directly overself.initial_values
. This is more Pythonic and avoids a redundant.keys()
call.-if any([self.is_partially_known(p) for p in self.initial_values.keys()]) \ - or any([other.is_partially_known(p) for p in other.initial_values.keys()]): +if any(self.is_partially_known(p) for p in self.initial_values) \ + or any(other.is_partially_known(p) for p in other.initial_values): return False🧰 Tools
🪛 Ruff (0.8.2)
295-302: Use a single
if
statement instead of nestedif
statements(SIM102)
299-299: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
301-301: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
295-302
: Combine conditions in a single if-statement.
You can simplify the nested OR conditions by merging them in one expression. This improves readability and aligns with the static analysis suggestion.🧰 Tools
🪛 Ruff (0.8.2)
295-302: Use a single
if
statement instead of nestedif
statements(SIM102)
299-299: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
301-301: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
examples/shakespeare-and-company-project/SCoData_events_v1.2_2022-01.csv
is excluded by!**/*.csv
📒 Files selected for processing (49)
.github/workflows/check.yml
(1 hunks).github/workflows/unit_tests.yml
(2 hunks)CHANGELOG.md
(1 hunks)README.md
(2 hunks)docs/undate/converters.rst
(2 hunks)docs/undate/core.rst
(1 hunks)examples/README.md
(1 hunks)examples/edtf-support.ipynb
(12 hunks)examples/ismi/README.md
(1 hunks)examples/ismi/data/ismi-crm-date-samples.ttl
(1 hunks)examples/notebooks/README.md
(0 hunks)examples/shakespeare-and-company-project/shxco_partial_date_durations.ipynb
(42 hunks)examples/use-cases/ismi/README.md
(0 hunks)examples/use-cases/ismi/data/ismi-om4-date-samples.json
(0 hunks)pyproject.toml
(3 hunks)src/undate/__init__.py
(1 hunks)src/undate/converters/base.py
(3 hunks)src/undate/converters/calendars/__init__.py
(1 hunks)src/undate/converters/calendars/gregorian.py
(1 hunks)src/undate/converters/calendars/hebrew/__init__.py
(1 hunks)src/undate/converters/calendars/hebrew/converter.py
(1 hunks)src/undate/converters/calendars/hebrew/hebrew.lark
(1 hunks)src/undate/converters/calendars/hebrew/parser.py
(1 hunks)src/undate/converters/calendars/hebrew/transformer.py
(1 hunks)src/undate/converters/calendars/islamic/__init__.py
(1 hunks)src/undate/converters/calendars/islamic/converter.py
(1 hunks)src/undate/converters/calendars/islamic/islamic.lark
(1 hunks)src/undate/converters/calendars/islamic/parser.py
(1 hunks)src/undate/converters/calendars/islamic/transformer.py
(1 hunks)src/undate/converters/edtf/converter.py
(1 hunks)src/undate/converters/edtf/edtf.lark
(1 hunks)src/undate/converters/edtf/parser.py
(1 hunks)src/undate/converters/edtf/transformer.py
(2 hunks)src/undate/converters/iso8601.py
(2 hunks)src/undate/interval.py
(1 hunks)src/undate/undate.py
(12 hunks)tests/test_converters/edtf/test_edtf_parser.py
(2 hunks)tests/test_converters/edtf/test_edtf_transformer.py
(1 hunks)tests/test_converters/test_base.py
(4 hunks)tests/test_converters/test_calendars/test_hebrew/test_hebrew_converter.py
(1 hunks)tests/test_converters/test_calendars/test_hebrew/test_hebrew_parser.py
(1 hunks)tests/test_converters/test_calendars/test_hebrew/test_hebrew_transformer.py
(1 hunks)tests/test_converters/test_calendars/test_islamic/test_islamic_converter.py
(1 hunks)tests/test_converters/test_calendars/test_islamic/test_islamic_parser.py
(1 hunks)tests/test_converters/test_calendars/test_islamic/test_islamic_transformer.py
(1 hunks)tests/test_converters/test_edtf.py
(2 hunks)tests/test_converters/test_iso8601.py
(1 hunks)tests/test_interval.py
(1 hunks)tests/test_undate.py
(5 hunks)
💤 Files with no reviewable changes (3)
- examples/notebooks/README.md
- examples/use-cases/ismi/README.md
- examples/use-cases/ismi/data/ismi-om4-date-samples.json
🧰 Additional context used
🧬 Code Graph Analysis (25)
src/undate/converters/calendars/hebrew/__init__.py (1)
src/undate/converters/calendars/hebrew/converter.py (1)
HebrewDateConverter
(12-78)
src/undate/converters/edtf/converter.py (3)
src/undate/undate.py (1)
Undate
(41-513)src/undate/converters/edtf/transformer.py (2)
EDTFTransformer
(6-75)date
(16-30)src/undate/date.py (1)
DatePrecision
(122-147)
tests/test_converters/edtf/test_edtf_transformer.py (2)
src/undate/undate.py (1)
Undate
(41-513)src/undate/interval.py (1)
UndateInterval
(10-181)
src/undate/converters/calendars/__init__.py (3)
src/undate/converters/calendars/gregorian.py (1)
GregorianDateConverter
(6-51)src/undate/converters/calendars/hebrew/converter.py (1)
HebrewDateConverter
(12-78)src/undate/converters/calendars/islamic/converter.py (1)
IslamicDateConverter
(12-67)
tests/test_converters/test_edtf.py (2)
src/undate/undate.py (1)
Undate
(41-513)src/undate/interval.py (1)
UndateInterval
(10-181)
tests/test_converters/test_iso8601.py (2)
src/undate/undate.py (1)
Undate
(41-513)src/undate/interval.py (1)
UndateInterval
(10-181)
src/undate/converters/edtf/transformer.py (2)
src/undate/undate.py (1)
Undate
(41-513)src/undate/interval.py (1)
UndateInterval
(10-181)
tests/test_converters/test_calendars/test_hebrew/test_hebrew_converter.py (4)
src/undate/converters/calendars/hebrew/converter.py (7)
HebrewDateConverter
(12-78)parse
(56-75)to_gregorian
(50-54)first_month
(36-38)last_month
(40-43)max_day
(45-48)max_month
(31-34)src/undate/converters/calendars/hebrew/transformer.py (1)
HebrewUndate
(6-9)src/undate/undate.py (4)
Calendar
(26-38)Undate
(41-513)parse
(231-239)year
(404-412)src/undate/date.py (2)
DatePrecision
(122-147)Date
(40-119)
src/undate/interval.py (5)
src/undate/undate.py (3)
Undate
(41-513)format
(241-249)duration
(443-480)src/undate/date.py (2)
Timedelta
(9-29)days
(27-29)src/undate/converters/base.py (2)
BaseDateConverter
(54-135)available_converters
(110-114)src/undate/converters/edtf/converter.py (1)
to_string
(55-67)src/undate/converters/iso8601.py (1)
to_string
(56-69)
src/undate/converters/calendars/islamic/transformer.py (1)
src/undate/undate.py (3)
Undate
(41-513)Calendar
(26-38)month
(415-425)
tests/test_converters/test_calendars/test_islamic/test_islamic_converter.py (5)
src/undate/converters/calendars/islamic/converter.py (3)
IslamicDateConverter
(12-67)parse
(45-64)to_gregorian
(39-43)src/undate/converters/calendars/islamic/transformer.py (1)
IslamicUndate
(6-9)src/undate/undate.py (4)
Calendar
(26-38)Undate
(41-513)parse
(231-239)year
(404-412)src/undate/date.py (2)
DatePrecision
(122-147)Date
(40-119)src/undate/converters/base.py (2)
parse
(61-69)to_gregorian
(165-169)
tests/test_converters/test_calendars/test_hebrew/test_hebrew_transformer.py (4)
src/undate/converters/calendars/hebrew/transformer.py (2)
HebrewDateTransformer
(12-40)HebrewUndate
(6-9)src/undate/undate.py (3)
Undate
(41-513)Calendar
(26-38)parse
(231-239)src/undate/date.py (1)
DatePrecision
(122-147)src/undate/converters/calendars/hebrew/converter.py (1)
parse
(56-75)
src/undate/converters/iso8601.py (1)
src/undate/undate.py (5)
Undate
(41-513)is_known
(391-398)year
(404-412)month
(415-425)day
(428-437)
src/undate/converters/calendars/islamic/__init__.py (1)
src/undate/converters/calendars/islamic/converter.py (1)
IslamicDateConverter
(12-67)
tests/test_converters/test_base.py (4)
src/undate/converters/base.py (7)
BaseDateConverter
(54-135)BaseCalendarConverter
(138-169)subclasses
(117-135)min_month
(145-147)max_month
(149-151)max_day
(161-163)to_gregorian
(165-169)src/undate/converters/calendars/gregorian.py (5)
GregorianDateConverter
(6-51)min_month
(19-21)max_month
(23-25)max_day
(27-43)to_gregorian
(45-51)src/undate/converters/calendars/hebrew/converter.py (1)
HebrewDateConverter
(12-78)src/undate/converters/calendars/islamic/converter.py (5)
IslamicDateConverter
(12-67)min_month
(31-33)max_month
(35-37)max_day
(27-29)to_gregorian
(39-43)
src/undate/__init__.py (4)
src/undate/converters/edtf/transformer.py (1)
date
(16-30)src/undate/date.py (1)
DatePrecision
(122-147)src/undate/undate.py (2)
Undate
(41-513)Calendar
(26-38)src/undate/interval.py (1)
UndateInterval
(10-181)
tests/test_converters/test_calendars/test_islamic/test_islamic_parser.py (3)
src/undate/converters/calendars/islamic/converter.py (1)
parse
(45-64)src/undate/converters/base.py (1)
parse
(61-69)src/undate/undate.py (1)
parse
(231-239)
tests/test_interval.py (2)
src/undate/interval.py (3)
UndateInterval
(10-181)duration
(84-124)intersection
(159-181)src/undate/date.py (2)
Timedelta
(9-29)days
(27-29)
src/undate/converters/calendars/islamic/converter.py (3)
src/undate/undate.py (5)
Undate
(41-513)year
(404-412)month
(415-425)day
(428-437)parse
(231-239)src/undate/converters/base.py (6)
BaseCalendarConverter
(138-169)max_day
(161-163)min_month
(145-147)max_month
(149-151)to_gregorian
(165-169)parse
(61-69)src/undate/converters/calendars/islamic/transformer.py (2)
IslamicDateTransformer
(12-40)month
(34-40)
tests/test_undate.py (2)
src/undate/undate.py (7)
Undate
(41-513)Calendar
(26-38)to_undate
(367-385)year
(404-412)month
(415-425)day
(428-437)get_converter
(34-38)src/undate/converters/base.py (1)
BaseCalendarConverter
(138-169)
src/undate/converters/base.py (3)
src/undate/converters/calendars/islamic/converter.py (4)
min_month
(31-33)max_month
(35-37)max_day
(27-29)to_gregorian
(39-43)src/undate/converters/calendars/hebrew/converter.py (6)
min_month
(27-29)max_month
(31-34)first_month
(36-38)last_month
(40-43)max_day
(45-48)to_gregorian
(50-54)src/undate/converters/calendars/gregorian.py (4)
min_month
(19-21)max_month
(23-25)max_day
(27-43)to_gregorian
(45-51)
src/undate/converters/calendars/hebrew/converter.py (3)
src/undate/undate.py (5)
Undate
(41-513)year
(404-412)month
(415-425)day
(428-437)parse
(231-239)src/undate/converters/base.py (8)
BaseCalendarConverter
(138-169)min_month
(145-147)max_month
(149-151)first_month
(153-155)last_month
(157-159)max_day
(161-163)to_gregorian
(165-169)parse
(61-69)src/undate/converters/calendars/hebrew/transformer.py (2)
HebrewDateTransformer
(12-40)month
(34-40)
tests/test_converters/test_calendars/test_islamic/test_islamic_transformer.py (3)
src/undate/converters/calendars/islamic/transformer.py (2)
IslamicDateTransformer
(12-40)IslamicUndate
(6-9)src/undate/date.py (1)
DatePrecision
(122-147)src/undate/converters/calendars/islamic/converter.py (1)
parse
(45-64)
src/undate/converters/calendars/hebrew/transformer.py (1)
src/undate/undate.py (3)
Undate
(41-513)Calendar
(26-38)month
(415-425)
src/undate/undate.py (4)
src/undate/converters/base.py (7)
BaseDateConverter
(54-135)available_converters
(110-114)min_month
(145-147)max_month
(149-151)max_day
(161-163)to_gregorian
(165-169)parse
(61-69)src/undate/converters/calendars/hebrew/converter.py (5)
min_month
(27-29)max_month
(31-34)max_day
(45-48)to_gregorian
(50-54)parse
(56-75)src/undate/converters/calendars/islamic/converter.py (5)
min_month
(31-33)max_month
(35-37)max_day
(27-29)to_gregorian
(39-43)parse
(45-64)src/undate/converters/calendars/gregorian.py (4)
min_month
(19-21)max_month
(23-25)max_day
(27-43)to_gregorian
(45-51)
🪛 LanguageTool
examples/ismi/README.md
[uncategorized] ~8-~8: Loose punctuation mark.
Context: ...crm:E52_Time-Span -
crm:P2_has_type`: date type - "datetype:day" - ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~17-~17: Loose punctuation mark.
Context: ...crm:E41_Appellation
- rdfs:label
: textual representation of timespan (e.g...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 Ruff (0.8.2)
tests/test_converters/test_calendars/test_islamic/test_islamic_parser.py
75-75: pytest.raises(Exception)
should be considered evil
(B017)
src/undate/undate.py
202-202: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
295-302: Use a single if
statement instead of nested if
statements
(SIM102)
299-299: Use key in dict
instead of key in dict.keys()
Remove .keys()
(SIM118)
301-301: Use key in dict
instead of key in dict.keys()
Remove .keys()
(SIM118)
🪛 markdownlint-cli2 (0.17.2)
README.md
184-184: Horizontal rule style
Expected: ---; Actual: * * *
(MD035, hr-style)
🔇 Additional comments (93)
src/undate/converters/calendars/hebrew/__init__.py (1)
1-3
: Good modular exposure of HebrewDateConverter.
The file cleanly imports theHebrewDateConverter
from its dedicated module and sets the public interface using__all__
. This approach clearly defines the export and follows standard Python module practices.CHANGELOG.md (1)
3-8
: Clear and descriptive changelog entry for v0.4.
The new version section details the enhancements by explicitly mentioning the support for Hebrew/Anno Mundi and Islamic/Hijri calendars. This change provides clarity on the new functionalities. Great job on updating the changelog to reflect these improvements.examples/edtf-support.ipynb (1)
1-800
: Consistent update of execution counts in the notebook.
The modifications adjust theexecution_count
values across multiple cells (e.g. changes from 2 to 1, 13 to 12, etc.), which is a non-functional but important tweak to maintain proper sequential ordering during re-execution. These updates ensure that the notebook’s cell order reflects a consistent execution history and do not interfere with the functional demonstration ofundate
features.src/undate/converters/calendars/islamic/__init__.py (1)
1-3
: Proper exposure of IslamicDateConverter.
This new module correctly importsIslamicDateConverter
from its correspondingconverter.py
and restricts the public API via the__all__
list. The implementation is straightforward and consistent with how similar converters are exposed in the codebase.src/undate/converters/calendars/__init__.py (1)
1-6
: Clean consolidation of calendar converters.
The file efficiently aggregates theGregorianDateConverter
,HebrewDateConverter
, andIslamicDateConverter
under one public module by importing them and defining them in the__all__
list. This centralizes the calendar converter APIs and supports easier imports in other parts of the library.src/undate/converters/edtf/edtf.lark (1)
19-19
: Improved day pattern for better validationThe updated regular expression now correctly handles days 01-09 with leading zeros and provides more precise matching for all valid day values from 01 to 31.
The new pattern breaks down as:
(0[1-9])
- matches days 01-09([12][0-9])
- matches days 10-29(3[01])
- matches days 30-31This is more accurate than the previous pattern and correctly validates day values.
src/undate/converters/edtf/converter.py (1)
5-5
: Simplified import pathThe import statement has been updated to import directly from the
undate
package instead of fromundate.undate
. This indicates that these classes are now exposed at the package level.This change simplifies the import statement and suggests that
Undate
andUndateInterval
are considered core components of the package that should be easily accessible.tests/test_converters/edtf/test_edtf_parser.py (2)
11-11
: Added test case for additional date formatGood addition of a specific date test case "1901-02-20" to improve coverage.
This test case helps ensure that the parser correctly handles a date from the early 20th century with a day in the "20s" range.
49-49
: Added important error cases for invalid datesThe addition of "1984-00" and "1984-01-00" as error cases is valuable for validating that the parser correctly rejects dates with zero month or zero day.
These test cases align with the improved day pattern in the grammar file and ensure that invalid dates are properly rejected by the parser.
tests/test_converters/edtf/test_edtf_transformer.py (1)
3-3
: Clean import structure improvement.The import statement has been simplified to import
Undate
andUndateInterval
directly from theundate
package rather than fromundate.undate
, reflecting a cleaner package organization where core classes are accessible from the top-level package.tests/test_converters/test_iso8601.py (1)
1-1
: Consistent import structure across the codebase.This change reflects the same pattern as in other files, importing
Undate
andUndateInterval
directly from the main package. This standardization improves code maintainability and readability.src/undate/converters/edtf/parser.py (1)
1-1
: Modern path handling with pathlib.The change from
os.path
topathlib
is a good modernization. Thepathlib
module provides an object-oriented approach to path manipulation that is more readable and maintainable. Using the/
operator for path construction is more intuitive thanos.path.join()
.Also applies to: 5-5
src/undate/converters/calendars/hebrew/parser.py (1)
1-9
: Well-structured Hebrew date parser implementation.The Hebrew date parser follows the same well-established pattern as other parsers in the codebase. The implementation:
- Uses modern
pathlib
for path handling- Includes a helpful comment about parser selection trade-offs
- Maintains appropriate configuration with
strict=True
This is a good addition to support the new Calendar API.
docs/undate/core.rst (3)
4-4
: Updated section title to reflect the new calendar functionality.The section title has been updated from "undates and undate intervals" to "dates, intervals, and calendar", which properly reflects the architectural changes with the introduction of the Calendar class.
10-12
: Added documentation for the new Calendar class.This addition properly documents the new Calendar class which provides support for different calendar systems (Gregorian, Hebrew, Islamic).
13-13
: Updated class import path for UndateInterval.The UndateInterval class has been moved from undate.undate to undate.interval module, and the documentation correctly reflects this change.
src/undate/__init__.py (2)
1-1
: Version number updated to 0.4.0Version bump from 0.3.1 to 0.4.0 is appropriate given the significant architectural changes with the introduction of the Calendar class and support for multiple calendar systems.
4-5
: Updated imports to reflect architectural changesThe imports have been correctly modified to:
- Import Calendar from undate.undate instead of UndateInterval
- Import UndateInterval from its new location in undate.interval
This reflects the reorganization of the codebase.
tests/test_converters/test_edtf.py (1)
3-3
: Updated import to use the relocated UndateIntervalThe import statement has been updated to properly import UndateInterval from its new location.
examples/ismi/README.md (1)
1-22
: Helpful documentation for ISMI project integrationThis README provides valuable context about how historical dates are stored in the ISMI project using CIDOC-CRM RDF objects, and how they relate to the different calendar types now supported by the undate library.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: Loose punctuation mark.
Context: ...crm:E52_Time-Span-
crm:P2_has_type`: date type - "datetype:day" - ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~17-~17: Loose punctuation mark.
Context: ...crm:E41_Appellation
-rdfs:label
: textual representation of timespan (e.g...(UNLIKELY_OPENING_PUNCTUATION)
examples/README.md (1)
3-13
: Great addition to documentation!The updated examples README now provides a much clearer structure with a dedicated "Contents" section and detailed descriptions of each resource. This makes it easier for users to navigate the examples and understand what each demonstrates.
README.md (2)
143-143
: LGTM: Updated converter descriptionThe text now correctly mentions support for calendars alongside ISO8601 and EDTF converters.
159-182
: Excellent calendar system documentationThe new Calendars section is well-structured and provides clear examples of how to use different calendar systems with Undate. The examples showing parsing of Hebrew and Islamic dates, along with the explanation of how dates are stored and compared across calendar systems, will be very helpful for users.
src/undate/converters/edtf/transformer.py (3)
3-3
: LGTM: Import statement simplifiedThe import statement has been simplified to directly import from the undate package.
57-58
: Improved code documentationAdding clarifying comments about the purpose of these methods makes the code more maintainable.
Also applies to: 62-64
69-69
: LGTM: Simplified year handlingRemoving the custom year method in favor of default transformation simplifies the code and aligns with the overall refactoring approach.
tests/test_converters/test_calendars/test_islamic/test_islamic_parser.py (2)
7-47
: Comprehensive test cases for Islamic date formatsThe test cases cover a wide variety of Islamic date formats including different representations of month names, with and without diacritics, and various digit configurations for years. This provides excellent test coverage for the Islamic date parser.
55-69
: Good error case coverageThe error cases thoroughly test various invalid inputs that the parser should reject, including invalid days, incomplete formats, and invalid month names.
tests/test_converters/test_base.py (4)
4-9
: Well-structured imports for the new calendar system tests.The imports correctly include the new
BaseCalendarConverter
class and the calendar converter implementations that will be tested.
26-26
: Method name change from__subclasses__()
tosubclasses()
.The change from using Python's built-in
__subclasses__()
to a customsubclasses()
method suggests an intentional change in how subclasses are retrieved, likely to control which classes are included in the results.Also applies to: 80-80
37-47
: Comprehensive test for the newsubclasses()
method.Good test that verifies:
- Nested subclasses are included correctly
BaseCalendarConverter
is excluded as expected- All calendar converter implementations are properly included
The test ensures that the inheritance hierarchy works as intended.
84-93
: Thorough test coverage forBaseCalendarConverter
abstract methods.The test correctly verifies that all abstract methods of
BaseCalendarConverter
raiseNotImplementedError
when called directly on the base class, enforcing the contract that subclasses must implement these methods..github/workflows/unit_tests.yml (5)
11-12
: Explicit branch specification for pull requests.The explicit branch specification is good practice for clarity, though the wildcard pattern ("**") effectively allows all branches, which was likely the default behavior before.
29-42
: Modernized CI setup with uv.The workflow now uses:
- Latest checkout action (v4)
- Latest setup-python action (v5)
- The
uv
package manager with cachingThis is a good modernization that should improve CI performance through better caching and more efficient dependency resolution.
43-44
: Simplified dependency installation.Using
uv sync --extra test
is a more concise way to install the package with test dependencies compared to the previouspip install -e ".[test]"
approach.
48-48
: Consistent command execution through uv.Using
uv run pytest
ensures that pytest runs in the same environment thatuv
created, which helps prevent potential environment inconsistencies.Also applies to: 53-53
23-23
:✅ Verification successful
Updated Python version matrix.
The matrix now includes Python 3.10-3.13 but removes 3.9. Make sure this aligns with the project's minimum Python version requirements defined in your package metadata.
🏁 Script executed:
#!/bin/bash # Check the minimum Python version specified in pyproject.toml grep -E "python.*=" pyproject.tomlLength of output: 84
Python version matrix aligns with package requirements.
- Verified that
pyproject.toml
specifiesrequires-python = ">= 3.10"
.- The updated workflow now tests Python versions 3.10, 3.11, 3.12, and 3.13, which correctly complies with the project's minimum requirements.
No further changes are needed.
tests/test_converters/test_calendars/test_islamic/test_islamic_transformer.py (5)
1-8
: Appropriate imports for Islamic calendar tests.The imports correctly include all the necessary components for testing the Islamic date transformer functionality.
11-12
: Basic verification of IslamicUndate calendar property.This test ensures that the calendar property is correctly set to
Calendar.ISLAMIC
forIslamicUndate
objects, which is a fundamental requirement.
15-35
: Comprehensive test cases for Islamic date parsing.The test cases include a good variety of Islamic date formats with:
- Different precisions (day, month, year)
- Different month names and formats
- Historical dates from real-world sources with conversion references
The commented line about parsing ranges suggests future functionality.
38-47
: Well-structured parameterized test for date transformation.The test properly validates:
- Correct parsing of the input string
- Transformation into the expected
IslamicUndate
object- Verification of date precision when applicable
Good use of pytest's parameterization to avoid test code duplication.
48-50
: Helpful comment about label assignment.The comment clarifies that labels are expected to be set by the converter class, not the transformer, which helps maintain clear separation of responsibilities.
src/undate/converters/calendars/islamic/transformer.py (4)
6-9
: Clean implementation of IslamicUndate.The
IslamicUndate
subclass is simple and effective, setting the default calendar toCalendar.ISLAMIC
at the class level.
12-27
: Well-implemented date transformation logic.The
islamic_date
method:
- Correctly extracts year, month, and day values from the parse tree
- Handles type conversion appropriately
- Uses keyword argument unpacking for clean instantiation of
IslamicUndate
This approach makes it easy to support partial dates (e.g., just year or year+month).
29-32
: Helpful comment about year transformation.The commented-out code with explanation helps future maintainers understand why a custom
year
method is not needed in this transformer.
34-40
: Clever month number extraction.The
month
method extracts the month number from the rule name (e.g.,month_1
,month_2
) instead of manually mapping month names to numbers, which is an elegant approach.tests/test_converters/test_calendars/test_hebrew/test_hebrew_transformer.py (4)
1-8
: Well organized imports with clear structure.The imports are properly organized, with standard library imports first (pytest), followed by application-specific imports. The specific import structure makes it clear what functionality is being tested.
11-12
: LGTM: The calendar type verification test is concise and effective.This test clearly verifies that the HebrewUndate correctly identifies itself with the Hebrew calendar type.
15-29
: Comprehensive test cases with well-documented examples.The test cases cover a good range of scenarios including different date precisions (year, month, day) and various formats of month names. The comments about the source of examples and validation using external tools provide excellent context for future maintainers.
The TODO comment for supporting date ranges is helpful for tracking future enhancements.
32-44
: Well-structured parametrized test with thorough assertions.The test function effectively validates both the transformation result and the precision of the date. The comments at the end about transformer limitations (not having access to the original date string) provide useful context about responsibilities in the system.
src/undate/converters/iso8601.py (2)
3-3
: LGTM: Updated import statement.The import has been updated to use a more direct import path from the package root, which follows best practices for package organization.
80-102
: Improved error handling for date formatting.The code now handles potential errors when converting date parts to integers, with appropriate comments explaining the defensive coding. This makes the code more robust against unexpected data formats.
While the comments mention that certain errors "shouldn't happen because of is_known", the defensive programming approach is still good practice for handling edge cases.
tests/test_converters/test_calendars/test_hebrew/test_hebrew_parser.py (5)
1-5
: LGTM: Clean imports and organization.The imports are clear and well-organized, importing only what's needed for the tests.
9-36
: Comprehensive test cases for valid Hebrew date formats.The test cases cover a wide range of valid formats including:
- Different date precisions (year, month+year, day+month+year)
- Variant month names with and without accents
- Special month handling (Adar I, Adar II, Adar Bet)
- Various year lengths (single digit, two digits, etc.)
This provides thorough coverage of the expected valid inputs.
39-41
: LGTM: Well-structured parameterized test for valid cases.The parameterized test is simple and effective, just verifying that parsing succeeds for valid inputs.
44-59
: Good error case coverage with specific expected exceptions.The error cases test important validation scenarios like:
- Invalid day values (0, 31 for certain months)
- Incomplete date formats
- Unsupported formats
- Reversed order formats
Each case specifies the exact exception type expected, which is good practice.
62-65
: LGTM: Effective error case validation.The test properly uses pytest's exception assertion mechanism to verify the parser raises the expected exceptions for invalid inputs.
examples/shakespeare-and-company-project/shxco_partial_date_durations.ipynb (3)
299-300
: Fixed file path for better portability.The update to use a relative path in the same directory makes the notebook more portable and easier to use without navigating to parent directories.
325-325
: Simplified import path for better API organization.The import statement has been updated to import UndateInterval directly from the undate package, which simplifies the import and suggests a more streamlined API organization.
24-35
: Updated execution count sequence.The notebook execution counts have been updated to ensure they are in sequential order starting from 1, which improves notebook readability and execution flow.
Also applies to: 44-56, 91-103, 144-156, 175-187, 240-254, 291-306, 342-357, 390-407, 426-451, 455-474, 508-525, 543-560, 584-601, 611-628, 631-651, 689-706, 730-750, 771-795, 835-852, 871-891, 911-928, 953-973, 992-1006, 1021-1045, 1065-1080, 1086-1104, 1116-1136, 1164-1182, 1202-1223, 1293-1364, 1380-1404, 1407-1427, 1439-1457, 1477-1491, 1537-1551, 1569-1583, 1606-1620, 1639-1653, 1679-1693, 1713-1727, 1731-1745, 1840-1854, 1873-1887, 1910-1924, 1965-1979, 1989-2014, 2032-2046, 2071-2084, 2094-2118, 2171-2185, 2192-2206, 2226-2240, 2318-2332, 2342-2356, 2383-2397, 2446-2460, 2469-2483, 2492-2506, 2522-2536, 2554-2568
src/undate/interval.py (1)
104-114
: Validate ambiguous year calculations more thoroughly.
When both years are unknown, the logic adds one year if the initial difference is negative. This approach may not handle edge cases where the date spans multiple years or months. Consider clarifying or strengthening assumptions about unknown-year scenarios to avoid accidental miscalculations, or raise an error when the interval cannot be inferred unambiguously.Do you want me to generate a script to search for any references to unknown-year intervals to ensure no conflicting assumptions exist?
src/undate/converters/calendars/hebrew/transformer.py (1)
16-27
: Confirm error handling forhebrew_date
method.
The transformer safely casts toint
, assuming well-formed parse tree items. If there's any chance of invalid or empty data, consider wrapping or verifying each field before converting. This helps avoid edge cases and ambiguous errors from Lark transformations.src/undate/converters/calendars/islamic/islamic.lark (2)
1-57
: Well-structured grammar for Islamic calendar datesThe grammar provides comprehensive support for Islamic date parsing with three formats (day-month-year, month-year, and year) and handles the complexity of Islamic month names well. The inclusion of month variants with and without accents will improve parsing robustness.
36-54
: Comprehensive month definitions for Islamic calendarThe month definitions are well-implemented with regular expressions that accommodate variations in spelling, accents, and prefixes (like "al-"). This approach will make the parser more robust when handling real-world data with different transliteration styles.
src/undate/converters/calendars/islamic/converter.py (3)
12-23
: Clean implementation of the IslamicDateConverter classThe class properly implements the BaseCalendarConverter interface with clear class attributes including name and calendar_name.
27-44
: Good use of external library for calendar calculationsThe implementation properly leverages the convertdate.islamic library for month length calculation and date conversion, avoiding unnecessary reimplementation of complex calendar math.
45-64
: Robust error handling in parse methodThe parse method includes proper error handling for empty strings and parsing failures, with clear error messages that will help with debugging.
src/undate/converters/calendars/gregorian.py (3)
6-18
: Clear implementation of GregorianDateConverterThe class is well-structured with appropriate documentation and a useful constant for non-leap year calculations.
27-44
: Robust max_day implementationThe max_day method handles different scenarios well, including:
- When both year and month are known
- When only month is known (using a non-leap year constant)
- When neither is known (returning maximum possible days)
Good use of the standard library's monthrange function for accurate calculations.
45-51
: Simple but effective to_gregorian implementationThe to_gregorian method appropriately returns the input parameters unchanged, maintaining consistency with the converter interface while acknowledging this is a no-op for the Gregorian calendar.
tests/test_converters/test_calendars/test_islamic/test_islamic_converter.py (5)
10-41
: Comprehensive tests for date parsing and conversionThe test_parse method thoroughly verifies various date formats (day-month-year, month-year, year) with appropriate assertions for calendar type, precision, and labeling.
42-75
: Thorough verification of Gregorian date conversionThe test_gregorian_earliest_latest method includes a good variety of test cases including:
- Specific days
- Month ranges
- Year ranges
- Real-world examples from historical data (ISMI)
These tests ensure the conversion logic works accurately across different time periods.
76-83
: Proper error case testingGood coverage of error conditions including invalid date strings and empty strings.
84-121
: Thorough testing of partially known datesExcellent test coverage for partially known dates, including unknown months, partially unknown months, unknown days, and partially unknown days. The tests verify both the precision level and the correct calculation of earliest/latest dates.
122-154
: Cross-calendar comparison testingThe test_compare_across_calendars method properly verifies that:
- Day-precision dates can be exactly equal across calendars
- Greater than/less than comparisons work correctly
- Containment checks function properly
- Sorting of mixed calendar dates produces the expected chronological order
This ensures the library maintains correct chronological relationships regardless of calendar system.
src/undate/converters/calendars/hebrew/hebrew.lark (2)
1-6
: Grammar implementation looks good.The grammar structure properly defines Hebrew dates with three supported formats (day-month-year, month-year, year), and correctly imports and ignores whitespace.
37-54
: Good handling of spelling variations.The regex patterns for month names effectively handle variations in spelling, particularly for months like Iyyar/Iyar and Tishrei/Tishri. This makes the parser more robust for real-world data.
tests/test_interval.py (3)
10-28
: Comprehensive type handling tests.The tests for initialization and type conversion properly verify both the auto-conversion of supported types and exception handling for unsupported types.
33-74
: Thorough string representation and formatting tests.The tests for string representation, formatting, and repr methods comprehensively cover various interval formats including open-ended intervals.
153-183
: Good test coverage for intersection logic.The tests thoroughly cover intersection calculations for different scenarios including non-overlapping intervals, fully contained intervals, partially overlapping intervals, and half-open intervals.
.github/workflows/check.yml (3)
20-26
: Good implementation of uv setup.The addition of the uv tool with caching enabled is a good improvement for dependency management. The cache-dependency-glob setting for pyproject.toml will help optimize CI performance.
31-44
: Consistent use of uv run across all tools.The consistent replacement of direct command invocation with uv run for all tools (ruff, sphinx-build, mypy, treon) ensures uniform execution environments and dependency resolution.
27-28
:✅ Verification successful
Check if sync needs --install-deps flag.
The uv sync command installs check dependencies, but you may want to verify if the --install-deps flag is needed to ensure all dependencies are properly installed.
🌐 Web query:
Does uv sync command require --install-deps flag?
💡 Result:
The
uv sync
command does not require the--install-deps
flag. By default,uv sync
automatically installs all project dependencies listed in the lockfile (uv.lock
) into the virtual environment[1][3][5].Key behaviors of
uv sync
:
- Automatic dependency installation: Installs all main dependencies from the lockfile by default, ensuring the environment matches the project's requirements[1][3].
- Dev dependencies: Includes the
dev
dependency group by default (use--no-dev
to exclude)[1][4].- Editable installs: Installs the current project in editable mode by default (use
--no-editable
to disable)[1].- Exact synchronization: Removes packages not in the lockfile by default (use
--inexact
to keep them)[1][5].Example basic usage:
uv sync # Installs all main + dev dependencies from lockfile uv sync --no-dev # Exclude development dependencies uv sync --extra docs # Include optional "docs" extrasThe
--install-deps
flag isn't referenced in uv's documentation because dependency installation is the primary purpose ofuv sync
. For advanced control, uv provides options like--extra
,--group
, and--no-dev
rather than requiring flags for basic functionality[1][4][5].Citations:
- 1: https://docs.astral.sh/uv/concepts/projects/sync/
- 2: https://compenv.phys.ethz.ch/python/ecosystem_1/45_uv/
- 3: https://www.saaspegasus.com/guides/uv-deep-dive/
- 4: https://docs.astral.sh/uv/concepts/projects/dependencies/
- 5: https://docs.astral.sh/uv/reference/cli/
- 6: https://www.datacamp.com/tutorial/python-uv
- 7: https://flocode.substack.com/p/044-python-environments-again-uv
- 8: Allow users to install a project's dependencies, without the project itself astral-sh/uv#4028
- 9: Expose more fine-grained control for
uv sync
command astral-sh/uv#5258- 10: https://calmcode.io/course/uv/sync
Clarification: The
--install-deps
flag is unnecessary.
Theuv sync
command automatically installs all dependencies from the lockfile by default. The current workflow step usinguv sync --extra check
is correct as written.docs/undate/converters.rst (3)
4-6
: Good addition of Overview section.Adding a dedicated Overview section improves the documentation structure and makes it easier for users to understand the converter system before diving into specific implementations.
11-31
: Well-organized Formats section.The reorganization to separate formats into a dedicated section with ISO8601 and EDTF subsections creates a logical hierarchy that makes the documentation more navigable.
33-52
: Comprehensive calendar converters documentation.The new Calendars section with dedicated subsections for Gregorian, Hebrew, and Islamic calendars provides clear organization and ensures users can easily find the specific calendar converter they need.
pyproject.toml (2)
10-10
: Verify dropping Python 3.9 support.Changing
requires-python
from ">=3.9" to ">=3.10" is a breaking change for users on Python 3.9. Confirm that all users and CI pipelines are able to meet this new requirement.
52-53
: Check for optional dependency overlap.You've introduced a new
notebooks
group and addedtreon
as well as a newcheck
group includingmypy
andruff
. Ensure these additions do not conflict or unnecessarily duplicate dependencies with thedev
group.src/undate/converters/base.py (2)
2-4
: Documentation improvements look good.This updated docstring clarifies the expanded scope for supporting both date converters and calendar converters. Nicely done.
138-170
: BaseCalendarConverter is well-structured.These abstract methods (e.g.,
min_month
,max_month
,max_day
) make it clear what calendar-specific converters must implement. This design neatly separates general date logic from calendar-specific logic.tests/test_undate.py (2)
122-132
: Good coverage for calendar tests.
Verifying different calendar assignment styles (string vs. enum) and the expected error for unsupported calendars is a thorough approach.
145-165
: Well-structuredtest_to_undate
method.
The method covers conversion from multiple object types (datetime.date
,datetime.datetime
, internalDate
), as well as error handling for invalid inputs. This test appears both robust and aligned with the newto_undate
logic. Nice job!examples/ismi/data/ismi-crm-date-samples.ttl (1)
74-82
: Verify existence of a Julian calendar converter.
The file defines Julian calendar date samples, but yourCalendar
enum only includesGREGORIAN
,HEBREW
, andISLAMIC
. Ensure you have or plan to implement the corresponding “JULIAN” converter if you intend to parse or convert these date samples programmatically.
import pathlib | ||
|
||
from lark import Lark | ||
|
||
grammar_path = pathlib.Path(__file__).parent / "islamic.lark" | ||
|
||
with open(grammar_path) as grammar: | ||
# NOTE: LALR parser is faster but can't be used due to ambiguity between years and days | ||
islamic_parser = Lark(grammar.read(), start="islamic_date", strict=True) |
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.
🛠️ Refactor suggestion
Code looks good, but consider adding error handling
The implementation correctly sets up a parser for Islamic dates using a grammar file in the same directory. The note about LALR parser limitations is helpful.
Consider adding error handling for file operations in case the grammar file is missing or inaccessible:
-with open(grammar_path) as grammar:
+try:
+ with open(grammar_path) as grammar:
+ # NOTE: LALR parser is faster but can't be used due to ambiguity between years and days
+ islamic_parser = Lark(grammar.read(), start="islamic_date", strict=True)
+except FileNotFoundError:
+ raise FileNotFoundError(f"Islamic calendar grammar file not found at {grammar_path}")
+except Exception as e:
+ raise RuntimeError(f"Error loading Islamic calendar grammar: {e}")
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import pathlib | |
from lark import Lark | |
grammar_path = pathlib.Path(__file__).parent / "islamic.lark" | |
with open(grammar_path) as grammar: | |
# NOTE: LALR parser is faster but can't be used due to ambiguity between years and days | |
islamic_parser = Lark(grammar.read(), start="islamic_date", strict=True) | |
import pathlib | |
from lark import Lark | |
grammar_path = pathlib.Path(__file__).parent / "islamic.lark" | |
try: | |
with open(grammar_path) as grammar: | |
# NOTE: LALR parser is faster but can't be used due to ambiguity between years and days | |
islamic_parser = Lark(grammar.read(), start="islamic_date", strict=True) | |
except FileNotFoundError: | |
raise FileNotFoundError(f"Islamic calendar grammar file not found at {grammar_path}") | |
except Exception as e: | |
raise RuntimeError(f"Error loading Islamic calendar grammar: {e}") |
@pytest.mark.parametrize("date_string", error_cases) | ||
def test_should_error(date_string): | ||
with pytest.raises(Exception): | ||
islamic_parser.parse(date_string) |
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.
🛠️ Refactor suggestion
Use more specific exception type
Using pytest.raises(Exception)
is too broad and could hide actual issues in the code. Based on the implementation in the converter, a ValueError
is raised for parsing errors.
@pytest.mark.parametrize("date_string", error_cases)
def test_should_error(date_string):
- with pytest.raises(Exception):
+ with pytest.raises(ValueError):
islamic_parser.parse(date_string)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
@pytest.mark.parametrize("date_string", error_cases) | |
def test_should_error(date_string): | |
with pytest.raises(Exception): | |
islamic_parser.parse(date_string) | |
@pytest.mark.parametrize("date_string", error_cases) | |
def test_should_error(date_string): | |
with pytest.raises(ValueError): | |
islamic_parser.parse(date_string) |
🧰 Tools
🪛 Ruff (0.8.2)
75-75: pytest.raises(Exception)
should be considered evil
(B017)
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
🧹 Nitpick comments (4)
README.md (1)
184-184
: Inconsistent horizontal rule styleThe horizontal rule style used here (
* * *
) differs from the standard style used elsewhere in the document (---
).-* * * +---🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
184-184: Horizontal rule style
Expected: ---; Actual: * * *(MD035, hr-style)
CHANGELOG.md (1)
22-22
: Remove empty bullet pointThere's an empty bullet point that should be removed.
--
src/undate/undate.py (2)
194-204
: Improve exception chaining in error handlingWhen raising exceptions inside an except block, it's better practice to use
raise ... from err
to maintain the exception chain for better debugging.try: calendar = Calendar[calendar.upper()] except KeyError as err: - raise ValueError(f"Calendar `{calendar}` is not supported") + raise ValueError(f"Calendar `{calendar}` is not supported") from err🧰 Tools
🪛 Ruff (0.8.2)
202-202: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
295-304
: Simplify nested conditional and improve dictionary lookupThe nested
if
can be flattened and the dictionary keys lookup can be simplified for better readability and performance.- if looks_equal: - # if any part of either date that is known is _partially_ known, - # then these dates are not equal - if any( - [self.is_partially_known(p) for p in self.initial_values.keys()] - ) or any( - [other.is_partially_known(p) for p in other.initial_values.keys()] - ): - return False + # If dates look equal but any part is partially known, they're not equal + if looks_equal and ( + any([self.is_partially_known(p) for p in self.initial_values]) or + any([other.is_partially_known(p) for p in other.initial_values]) + ): + return False🧰 Tools
🪛 Ruff (0.8.2)
295-302: Use a single
if
statement instead of nestedif
statements(SIM102)
299-299: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
301-301: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md
(2 hunks)README.md
(3 hunks)src/undate/undate.py
(12 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/undate/undate.py (3)
src/undate/interval.py (2)
UndateInterval
(10-181)format
(63-70)src/undate/converters/base.py (3)
BaseDateConverter
(54-135)available_converters
(110-114)parse
(61-69)src/undate/date.py (5)
Date
(40-119)Timedelta
(9-29)year
(90-91)month
(94-98)day
(101-105)
🪛 markdownlint-cli2 (0.17.2)
README.md
184-184: Horizontal rule style
Expected: ---; Actual: * * *
(MD035, hr-style)
🪛 Ruff (0.8.2)
src/undate/undate.py
202-202: Within an except
clause, raise exceptions with raise ... from err
or raise ... from None
to distinguish them from errors in exception handling
(B904)
295-302: Use a single if
statement instead of nested if
statements
(SIM102)
299-299: Use key in dict
instead of key in dict.keys()
Remove .keys()
(SIM118)
301-301: Use key in dict
instead of key in dict.keys()
Remove .keys()
(SIM118)
🔇 Additional comments (8)
README.md (2)
7-10
: Updated warning status reflects maturity progressThe warning section has been appropriately updated to indicate that the software is now in alpha stage rather than pre-alpha, which accurately reflects the library's maturity progression. The expanded description of supported date formats now includes the new calendar capabilities.
159-182
: Well-documented calendar functionality with clear examplesThe new "Calendars" section provides excellent documentation for the calendar-aware functionality. The examples clearly demonstrate how to work with different calendar systems and how dates from different calendars can be sorted and compared using the Gregorian calendar as a common reference.
CHANGELOG.md (1)
3-21
: Comprehensive changelog for significant calendar functionalityThe changelog effectively documents all the significant enhancements in version 0.4, particularly the new calendar functionality and improvements to date conversion and interval handling.
src/undate/undate.py (5)
26-38
: Well-implemented Calendar enum with converter lookup methodThe new Calendar enum provides a clear structure for supported calendar systems with a convenient static method to retrieve the appropriate converter.
77-80
: Improved input validation for Undate initializationAdding validation to ensure at least one date component is specified is a good safety check that prevents creating empty or meaningless Undate objects.
137-144
: Calendar-aware month range calculationThe implementation now properly handles calendar-specific month ranges, which is crucial for non-Gregorian calendars that may have different month structures.
185-192
: Standardized comparison with Gregorian conversionConverting dates to the Gregorian calendar for earliest/latest calculations ensures consistent and accurate comparisons across different calendar systems. This approach maintains the original calendar for display while enabling proper date arithmetic.
366-386
: Excellent to_undate implementation with pattern matchingThe new
to_undate
method provides a clean, centralized way to convert various date objects to Undate instances. The use of pattern matching (introduced in Python 3.10) makes the code more readable and maintainable.Note that this approach requires Python 3.10+, which aligns with dropping support for Python 3.9 as mentioned in the changelog.
# TODO: test this method directly | ||
|
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.
💡 Verification agent
❓ Verification inconclusive
Add unit tests for _missing_digit_minmax method
The TODO comment correctly identifies that this method should have direct unit tests. Consider making this a priority since it's a critical part of the date range calculation logic.
🏁 Script executed:
#!/bin/bash
# Check if there are any existing tests for the _missing_digit_minmax method
rg -A 3 -B 3 "_missing_digit_minmax" -g "test_*.py" ./tests/
Length of output: 60
Action Required: Add Direct Unit Tests for the _missing_digit_minmax Method
It appears that there are currently no tests targeting the _missing_digit_minmax method in the tests directory. This method plays a critical role in the date range calculation logic, so creating focused unit tests should be a priority. Once the tests are added, please update or remove the # TODO: test this method directly
comment accordingly.
- File:
src/undate/undate.py
(Lines 489-490) - Next Steps: Implement unit tests that specifically validate the behavior of _missing_digit_minmax to ensure robust date range calculations.
Summary by CodeRabbit
New Features
BaseCalendarConverter
class for improved calendar conversion functionality.UndateInterval
to represent date ranges with enhanced error handling.Undate
class to support calendar awareness and improved date handling.Refactor
Documentation