Revise __repr__ methods to return strings that can be used with eval() #144
Conversation
|
Warning Rate limit exceeded@rlskoeser has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 48 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds Changes
Sequence Diagram(s)(omitted — changes are representation/output formatting only, no control-flow or runtime interaction changes) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
rlskoeser
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #144 +/- ##
===========================================
- Coverage 98.72% 98.69% -0.04%
===========================================
Files 38 38
Lines 1956 1986 +30
===========================================
+ Hits 1931 1960 +29
- Misses 25 26 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
tests/test_date.py (1)
233-247: Strengthen repr round-trip test to cover whole objectYou validate endpoints and module import; also assert eval(repr(obj)) reconstructs an UnDelta with the same bounds.
Apply:
feb_undelt = UnDelta(28, 29) assert repr(feb_undelt) == "undate.UnDelta(28,29)" # can't compare directly because uncertain deltas aren't equal, - # but compare vlaues + # but compare values assert eval(repr(feb_undelt.days.lower)) == feb_undelt.days.lower assert eval(repr(feb_undelt.days.upper)) == feb_undelt.days.upper + # round-trip the whole object + rt = eval(repr(feb_undelt)) + assert isinstance(rt, UnDelta) + assert (rt.days.lower, rt.days.upper) == (feb_undelt.days.lower, feb_undelt.days.upper)tests/test_interval.py (4)
59-61: Pass explicit globals to eval() for clarity and safetyRelying on a function-scope import to make
undatevisible toeval()works, but is brittle. Passing an explicit globals dict is clearer and avoids surprises.- assert eval(repr(closed_interval)) == closed_interval + assert eval(repr(closed_interval), {"undate": undate}) == closed_interval @@ - assert eval(repr(fancy_epoch)) == fancy_epoch + assert eval(repr(fancy_epoch), {"undate": undate}) == fancy_epoch @@ - assert eval(repr(open_interval)) == open_interval + assert eval(repr(open_interval), {"undate": undate}) == open_intervalAlso applies to: 69-69, 76-76, 85-85
63-67: Reduce brittleness of exact repr string assertionsExact string equality ties tests to argument ordering/spacing decisions. Since eval-roundtrip is already asserted, consider relaxing these to structural checks (e.g., startswith, substrings) or keep only the eval equality.
Also applies to: 73-75, 82-84
71-76: Add a label-with-quotes case to ensure proper escaping in reprExercise escaping to guarantee eval(repr(...)) survives embedded quotes and non-ASCII.
fancy_epoch = UndateInterval(Undate(2022), Undate(2023), label="Fancy Epoch") assert ( repr(fancy_epoch) == f'undate.UndateInterval(earliest={repr(fancy_epoch.earliest)}, latest={repr(fancy_epoch.latest)}, label="Fancy Epoch")' ) - assert eval(repr(fancy_epoch)) == fancy_epoch + assert eval(repr(fancy_epoch), {"undate": undate}) == fancy_epoch + + tricky_label = 'Fancy "Epoch" — v2' + tricky = UndateInterval(Undate(2022), Undate(2023), label=tricky_label) + assert eval(repr(tricky), {"undate": undate}) == tricky
78-85: Also cover latest-only (open-start) reprYou cover earliest-only; mirror it with a latest-only case for symmetry.
open_interval = UndateInterval( Undate(33), ) assert ( repr(open_interval) == f"undate.UndateInterval(earliest={repr(open_interval.earliest)})" ) - assert eval(repr(open_interval)) == open_interval + assert eval(repr(open_interval), {"undate": undate}) == open_interval + + latest_only = UndateInterval(latest=Undate(2023)) + assert ( + repr(latest_only) + == f"undate.UndateInterval(latest={repr(latest_only.latest)})" + ) + assert eval(repr(latest_only), {"undate": undate}) == latest_onlytests/test_undate.py (3)
31-34: Bind eval() to the undate module explicitlyMake the dependency on
undateexplicit and avoid relying on scope rules. Apply to all eval calls in this test.- assert eval(repr(nov2022)) == nov2022 + assert eval(repr(nov2022), {"undate": undate}) == nov2022 @@ - assert eval(repr(nov2022_labeled)) == nov2022_labeled + assert eval(repr(nov2022_labeled), {"undate": undate}) == nov2022_labeled @@ - assert eval(repr(islamic_date)) == islamic_date + assert eval(repr(islamic_date), {"undate": undate}) == islamic_date @@ - assert str(eval(repr(unknown_year))) == str(unknown_year) + assert str(eval(repr(unknown_year), {"undate": undate})) == str(unknown_year)Also applies to: 42-42, 48-48, 52-52, 61-61
44-49: Add a label-with-quotes/non-ASCII caseEnsure proper escaping and eval-roundtrip when labels contain quotes or Unicode.
nov2022_labeled = Undate(2022, 11, 7, label="A Special Day") assert ( repr(nov2022_labeled) == 'undate.Undate(year=2022, month=11, day=7, label="A Special Day", calendar="Gregorian")' ) - assert eval(repr(nov2022_labeled)) == nov2022_labeled + assert eval(repr(nov2022_labeled), {"undate": undate}) == nov2022_labeled + + tricky_label = 'A "Special" Day — ©' + tricky = Undate(2022, 11, 7, label=tricky_label) + assert eval(repr(tricky), {"undate": undate}) == tricky
54-62: Assert repr idempotency for unknown componentsUnknown parts aren’t equal, but repr should still be stable across a round-trip.
- assert str(eval(repr(unknown_year))) == str(unknown_year) + assert str(eval(repr(unknown_year), {"undate": undate})) == str(unknown_year) + assert repr(eval(repr(unknown_year), {"undate": undate})) == repr(unknown_year)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/undate/__init__.py(1 hunks)src/undate/date.py(1 hunks)src/undate/interval.py(1 hunks)src/undate/undate.py(1 hunks)tests/test_date.py(1 hunks)tests/test_interval.py(1 hunks)tests/test_undate.py(1 hunks)
🔇 Additional comments (5)
src/undate/date.py (1)
145-149: LGTM: eval-friendly UnDelta.reprFormat is deterministic and round-trips via eval with endpoints only. Matches PR intent.
src/undate/undate.py (1)
253-263: Confirm intent: converter not preserved in reprRound-tripping via eval() will always construct with the default converter, losing any non-default converter passed at init. If that’s intentional, consider documenting it; otherwise, we can include a converter hint (e.g., by name) and extend init to accept it.
Do you want me to draft a follow-up that accepts converter="ISO8601" (by name) in init and incorporates it into repr?
src/undate/__init__.py (1)
3-14: LGTM: UnDelta exported at package rootThis enables undate.UnDelta in repr strings and eval round-trips as intended.
tests/test_undate.py (2)
35-43: LGTM: canonical, eval-friendly repr for fully-known Gregorian dateThe exact string and round-trip checks look solid.
50-53: LGTM: alternate calendar repr is round-trippableGood coverage of non-Gregorian calendar emission and reconstruction.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
README.md (2)
124-138: Use the standard code-fence language for REPL examples ("pycon")."Python console" is not a recognized lexer on most renderers; use "pycon" for correct highlighting of >>> prompts.
-```Python console +```pycon
187-198: Minor formatting nit in REPL prompt spacing.There’s an extra space after the REPL prompt on Line 196; remove for consistency.
->>> Undate.parse("1800/1900", format="EDTF") +>>> Undate.parse("1800/1900", format="EDTF")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md(4 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~236-~236: There might be a mistake here.
Context: ...date-python/tree/main/examples/) in this repository. ## Documentation Project d...
(QB_NEW_EN)
🔇 Additional comments (2)
README.md (2)
161-175: Repr outputs look correct and align with eval-friendly constructor style.Interval examples now show fully-qualified undate.UndateInterval(...) with nested undate.Undate(...); consistent with the PR goal.
215-231: Calendar examples and sorted output match new repr semantics.Objects display explicit calendar and labels; sort order aligns with earliest dates across calendars.
rlskoeser
force-pushed
the
feature/revise-repr
branch
from
3656437 to
8b30880
Compare
To address #142
__repr__methods forUndate,UndateInterval, andUnDeltato return a string that documents object initialization options and can be used witheval()eval()results in an equivalent objectSummary by CodeRabbit
New Features
Refactor
Tests
Documentation