-
Notifications
You must be signed in to change notification settings - Fork 3
Release v0.6 #150
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.6 #150
Conversation
chore: update workflow actions
docs: add rettinghaus as a contributor for maintenance
Experimental combined / omnibus date parser
WalkthroughAdds version 0.6.0, introduces an experimental OmnibusDateConverter with a combined grammar (EDTF, Hebrew, Islamic), centralizes grammar path via GRAMMAR_FILE_PATH, updates calendar transformers/parsers, bumps GitHub Action versions, updates docs, tests, changelog, and contributor metadata. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Omnibus as OmnibusDateConverter
participant Parser as Lark Parser
participant Transformer as CombinedTransformer
participant Calendar as Individual Transformers
participant Undate
Client->>Omnibus: parse(date_string)
Omnibus->>Parser: parse(date_string) using combined.lark
Parser-->>Omnibus: parse_tree
Omnibus->>Transformer: transform(parse_tree)
Transformer->>Calendar: delegate calendar-specific rules (year/month/day)
Calendar-->>Transformer: transformed subtrees
Transformer-->>Omnibus: Undate / UndateInterval
Omnibus-->>Client: return Undate / UndateInterval
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✏️ Tip: You can disable this entire section by setting 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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #150 +/- ##
==========================================
+ Coverage 98.93% 98.96% +0.03%
==========================================
Files 38 40 +2
Lines 2057 2121 +64
==========================================
+ Hits 2035 2099 +64
Misses 22 22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/unit_tests.yml (1)
33-57: Runner version requirement for Node.js 24 compatibility.Both actions/checkout@v5 and actions/setup-python@v6 require GitHub Actions Runner v2.327.1 or newer due to Node.js 24 runtime. GitHub-hosted runners meet this requirement; self-hosted runners must be updated.
codecov-action@v5 is compatible with this workflow's current configuration—it doesn't use deprecated inputs (
file,plugin) and has no breaking changes in the parameters shown..github/workflows/python-publish.yml (1)
36-36: Update the pinnedpypa/gh-action-pypi-publishSHA to a recent release.The current SHA (
27b31702a0e7fc50959f5ad993c78deac1bdfc29) is from February 2021 and predates all published releases. The latest stable version is v1.13.0 (September 2025), with v1.12.4 also available (January 2025). Updating to a recent release will include security patches and feature improvements.
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Around line 3-8: Update the typo "Hirji" to "Hijri" in the changelog entry
under "## 0.6" (the line "- Experimental omnibus date converter + parser (EDTF,
Hebrew, Hirji)") so the phrase reads "(EDTF, Hebrew, Hijri)"; locate and edit
that exact string in CHANGELOG.md.
🧹 Nitpick comments (1)
docs/undate/converters.rst (1)
46-47: Consider adding:members:directive.The
automoduleforundate.converters.calendarsdoesn't include:members:, which means it will only render the module docstring. If there are public members to document, consider adding:members:and:undoc-members:for consistency with other module documentation above.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
.all-contributorsrc.github/workflows/check.yml.github/workflows/python-publish.yml.github/workflows/unit_tests.ymlCHANGELOG.mdCONTRIBUTORS.mddocs/undate/converters.rstsrc/undate/__init__.pysrc/undate/converters/__init__.pysrc/undate/converters/base.pysrc/undate/converters/calendars/hebrew/parser.pysrc/undate/converters/calendars/hebrew/transformer.pysrc/undate/converters/calendars/islamic/parser.pysrc/undate/converters/calendars/islamic/transformer.pysrc/undate/converters/combined.pysrc/undate/converters/edtf/parser.pysrc/undate/converters/edtf/transformer.pysrc/undate/converters/grammars/combined.larksrc/undate/converters/grammars/edtf.larksrc/undate/converters/grammars/hebrew.larksrc/undate/converters/grammars/islamic.larktests/test_converters/test_combined_parser.py
🧰 Additional context used
🧬 Code graph analysis (6)
src/undate/converters/calendars/islamic/transformer.py (1)
src/undate/undate.py (2)
year(484-492)day(508-517)
src/undate/converters/combined.py (6)
src/undate/undate.py (1)
Undate(51-678)src/undate/interval.py (1)
UndateInterval(10-189)src/undate/converters/base.py (3)
BaseDateConverter(61-146)parse(72-80)to_string(82-91)src/undate/converters/edtf/transformer.py (1)
EDTFTransformer(6-78)src/undate/converters/calendars/hebrew/transformer.py (1)
HebrewDateTransformer(12-43)src/undate/converters/calendars/islamic/transformer.py (1)
IslamicDateTransformer(12-49)
src/undate/converters/edtf/transformer.py (2)
src/undate/converters/calendars/hebrew/transformer.py (1)
year(32-35)src/undate/undate.py (1)
year(484-492)
tests/test_converters/test_combined_parser.py (3)
src/undate/undate.py (1)
Undate(51-678)src/undate/converters/combined.py (1)
parse(65-81)src/undate/converters/base.py (1)
parse(72-80)
src/undate/converters/__init__.py (1)
src/undate/converters/base.py (1)
BaseDateConverter(61-146)
src/undate/converters/calendars/hebrew/transformer.py (2)
src/undate/undate.py (2)
Undate(51-678)year(484-492)src/undate/converters/edtf/transformer.py (1)
year(69-72)
🔇 Additional comments (20)
src/undate/__init__.py (1)
1-1: Version bump looks good.CONTRIBUTORS.md (1)
23-23: Contributor addition looks good.src/undate/converters/grammars/hebrew.lark (1)
14-30: Grammar cleanup looks fine..github/workflows/check.yml (1)
18-26: No breaking changes affect this workflow configuration.The versions bump introduces breaking changes in astral-sh/setup-uv v6 (removed pyproject-file and uv-file inputs) and runtime upgrades in both core actions (Node.js 24, requiring runner v2.327.1+), but this workflow avoids all removed inputs and explicitly specifies cache-dependency-glob, preventing issues from changed defaults. GitHub-hosted runners (ubuntu-latest) are auto-updated to v2.327.1+, and no additional permissions are needed.
.all-contributorsrc (1)
81-90: LGTM!The new contributor entry for Klaus Rettinghaus follows the established JSON structure correctly. The "maintenance" contribution type appropriately reflects the GitHub Actions workflow version updates documented in the changelog.
src/undate/converters/edtf/transformer.py (1)
69-72: LGTM!The new
yearmethod correctly mirrors the existingyear_unspecifiedimplementation pattern. Usingget_valuesto recursively extract tokens from subtrees is appropriate here since EDTF year rules may produce nested tree structures (e.g., for negative years with sign prefixes), unlike the simpler Hebrew transformer that receives flat items..github/workflows/python-publish.yml (1)
24-26: LGTM on the action version updates.The upgrades to
actions/checkout@v5andactions/setup-python@v6are appropriate and align with current best practices.src/undate/converters/grammars/combined.lark (1)
1-32: LGTM!The combined grammar is well-structured:
- Proper namespace prefixing for imported rules avoids conflicts
- The override rules appropriately restrict Hebrew and Islamic dates to require at least month+year, preventing ambiguous year-only parsing where calendar context would be unclear
- Good documentation via comments explaining the rationale for renaming imports and the year-only restriction
The comment noting potential future support for "year with calendar label" (line 28) is a helpful breadcrumb for future enhancement.
src/undate/converters/edtf/parser.py (1)
3-5: LGTM!Clean refactor to use the centralized
GRAMMAR_FILE_PATHconstant. This aligns with the consistent grammar file resolution strategy across all calendar parsers.src/undate/converters/base.py (1)
47-60: LGTM!Good centralization of the grammar file path. Using
pathlib.Path(__file__).parentis the idiomatic approach for locating resources relative to the module. The Sphinx-style docstring (#:) properly documents the constant for the API documentation.src/undate/converters/calendars/islamic/transformer.py (1)
31-41: LGTM!The
yearanddaymethods correctly handle tokenized input from the combined parser by joining the parts into a single string. The downstreamislamic_datemethod properly casts these string values to integers viaint(child.children[0]).The inline comments explaining the "anonymous token in combined parser" reasoning are helpful for future maintainers.
docs/undate/converters.rst (1)
7-19: LGTM!Documentation properly updated to include the new
OmnibusDateConverterand the converters module overview. The Sphinx directives are correctly structured.src/undate/converters/calendars/islamic/parser.py (1)
3-5: LGTM!Consistent refactor to use the centralized
GRAMMAR_FILE_PATHconstant, matching the pattern applied to the EDTF parser.src/undate/converters/__init__.py (1)
1-29: LGTM!The module docstring provides clear usage examples, and the centralized export of
GRAMMAR_FILE_PATHalongsideBaseDateConvertercleanly supports the grammar path resolution refactor across parser modules.src/undate/converters/calendars/hebrew/parser.py (1)
3-5: LGTM!Clean refactor to use the centralized
GRAMMAR_FILE_PATHconstant, consistent with the pattern applied across other parser modules.src/undate/converters/calendars/hebrew/transformer.py (1)
32-35: LGTM!The new
year()method follows the same pattern asIslamicDateTransformer.year(), which is appropriate since both calendar transformers handle simpler token structures compared to EDTF's recursiveget_values()approach. The implementation is consistent with the sibling transformer.tests/test_converters/test_combined_parser.py (1)
1-54: LGTM!Comprehensive test coverage for the omnibus converter:
- Tests both direct transformer usage and the
Undate.parse()API path- Covers EDTF, Hebrew, and Islamic calendar formats
- Validates error handling for empty strings and unrecognized formats
- Confirms serialization raises the expected error
The
repr()comparison approach is well-documented and appropriate given the equality semantics for partially unknown dates.src/undate/converters/combined.py (3)
45-85: LGTM on the OmnibusDateConverter structure.The converter class is well-documented with clear docstring, appropriate
nameattribute for registration, and explicit handling for unsupported serialization. The "experimental" designation in both the module and class docstrings appropriately sets expectations.
40-42: Therel_toparameter is necessary and should not be removed.The
combined.larkgrammar uses multiple relative imports (.edtf,.hebrew,.islamic) that require therel_to=__file__parameter to resolve correctly. Lark uses this parameter as the base path for resolving%importstatements with relative paths. Removingrel_toor switching to a directLark()constructor would break these relative imports.Likely an incorrect or invalid review comment.
74-81: This exception handling pattern is consistent with the codebase and doesn't require changes.The review suggests catching
UnexpectedToken, but the codebase consistently catches onlyUnexpectedCharactersacross all converter modules (combined, edtf, hebrew, islamic). No tests or code in the repository indicate thatUnexpectedTokenis raised during parsing with these grammars. The existing exception handling is aligned with the established pattern and should be left as-is.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.