Skip to content

Conversation

@rlskoeser
Copy link
Member

@rlskoeser rlskoeser commented Jan 16, 2026

Summary by CodeRabbit

  • New Features

    • Experimental Omnibus date parser added to parse EDTF, Hebrew, and Islamic formats together.
    • Grammar files reorganized to a centralized location for easier maintenance.
  • Documentation

    • Improved converter parsing and serialization docs and added references for converters.
  • Chores

    • Project version updated to 0.6.0.
    • CI workflows updated to newer action versions.
    • New maintainer added to contributors.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 16, 2026

Walkthrough

Adds 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

Cohort / File(s) Summary
Contributor & Release Docs
\.all-contributorsrc, CONTRIBUTORS.md, CHANGELOG.md
Adds contributor rettinghaus (Klaus Rettinghaus) and documents v0.6.0 release entries.
Version & Exports
src/undate/__init__.py, src/undate/converters/__init__.py, src/undate/converters/base.py
Bumps package version to 0.6.0; adds GRAMMAR_FILE_PATH constant and exports it from converters package.
GitHub Actions
.github/workflows/check.yml, .github/workflows/python-publish.yml, .github/workflows/unit_tests.yml
Bumps action versions (actions/checkout → v5, actions/setup-python → v6, astral-sh/setup-uv → v6, codecov-action → v5).
Grammar Path Refactor
src/undate/converters/edtf/parser.py, src/undate/converters/calendars/hebrew/parser.py, src/undate/converters/calendars/islamic/parser.py
Replace local pathlib-based grammar resolution with centralized GRAMMAR_FILE_PATH / "<name>.lark".
Combined Grammar & Converter
src/undate/converters/grammars/combined.lark, src/undate/converters/combined.py
Adds combined.lark; implements OmnibusDateConverter that parses via combined grammar and transforms to Undate/intervals; serialization unsupported.
Transformer Enhancements
src/undate/converters/edtf/transformer.py, src/undate/converters/calendars/hebrew/transformer.py, src/undate/converters/calendars/islamic/transformer.py
Add concrete year() (EDTF, Hebrew, Islamic) and day() (Islamic) transformer methods to build Tree nodes from token parts.
Docs & Tests
docs/undate/converters.rst, tests/test_converters/test_combined_parser.py
Adds autodoc references for converters; introduces tests validating combined parser/transformer behavior and error cases.
Grammar minor edit
src/undate/converters/grammars/hebrew.lark
Minor comment/formatting adjustments only.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • PR #112: Implements the omnibus/combined date parser and related transformer/grammar changes overlapping with this PR.
  • PR #147: Mirrors workflow action version bumps in the same workflow files.
  • PR #125: Touches Hebrew/Islamic transformer changes (year/day) and related parser adjustments.

Suggested reviewers

  • robcast
  • ColeDCrawford

Poem

🐰 Hopped through grammars, stitched three into one,
I danced with years and days until parsing was done.
A bump, a test, a contributor cheer—
Omnibus blooms and workflows gear.
Hoppity hop, release draws near! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Release v0.6' directly and clearly corresponds to the main purpose of the changeset: releasing version 0.6 of the undate package with version bump, changelog updates, and new features.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.



📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9e9da0c and 1f0f7c0.

📒 Files selected for processing (1)
  • CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Jan 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.96%. Comparing base (5ef75b4) to head (1f0f7c0).
⚠️ Report is 18 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 pinned pypa/gh-action-pypi-publish SHA 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 automodule for undate.converters.calendars doesn'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

📥 Commits

Reviewing files that changed from the base of the PR and between 5ef75b4 and 9e9da0c.

📒 Files selected for processing (22)
  • .all-contributorsrc
  • .github/workflows/check.yml
  • .github/workflows/python-publish.yml
  • .github/workflows/unit_tests.yml
  • CHANGELOG.md
  • CONTRIBUTORS.md
  • docs/undate/converters.rst
  • src/undate/__init__.py
  • src/undate/converters/__init__.py
  • src/undate/converters/base.py
  • src/undate/converters/calendars/hebrew/parser.py
  • src/undate/converters/calendars/hebrew/transformer.py
  • src/undate/converters/calendars/islamic/parser.py
  • src/undate/converters/calendars/islamic/transformer.py
  • src/undate/converters/combined.py
  • src/undate/converters/edtf/parser.py
  • src/undate/converters/edtf/transformer.py
  • src/undate/converters/grammars/combined.lark
  • src/undate/converters/grammars/edtf.lark
  • src/undate/converters/grammars/hebrew.lark
  • src/undate/converters/grammars/islamic.lark
  • tests/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 year method correctly mirrors the existing year_unspecified implementation pattern. Using get_values to 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@v5 and actions/setup-python@v6 are 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_PATH constant. 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__).parent is 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 year and day methods correctly handle tokenized input from the combined parser by joining the parts into a single string. The downstream islamic_date method properly casts these string values to integers via int(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 OmnibusDateConverter and 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_PATH constant, 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_PATH alongside BaseDateConverter cleanly 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_PATH constant, 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 as IslamicDateTransformer.year(), which is appropriate since both calendar transformers handle simpler token structures compared to EDTF's recursive get_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 name attribute for registration, and explicit handling for unsupported serialization. The "experimental" designation in both the module and class docstrings appropriately sets expectations.


40-42: The rel_to parameter is necessary and should not be removed.

The combined.lark grammar uses multiple relative imports (.edtf, .hebrew, .islamic) that require the rel_to=__file__ parameter to resolve correctly. Lark uses this parameter as the base path for resolving %import statements with relative paths. Removing rel_to or switching to a direct Lark() 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 only UnexpectedCharacters across all converter modules (combined, edtf, hebrew, islamic). No tests or code in the repository indicate that UnexpectedToken is 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.

@rlskoeser rlskoeser merged commit d42018f into main Jan 16, 2026
13 checks passed
@rlskoeser rlskoeser deleted the release/0.6 branch January 16, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants