✨ Rename the dump-side tags= registry to serializers=#222
Conversation
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request renames the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant async_dump
participant dump
participant dumps_rs as dumps (Rust FFI)
participant EncodeCtx
Caller->>async_dump: async_dump(obj, serializers={...})
async_dump->>dump: dump(obj, serializers=serializers)
dump->>dumps_rs: dumps(obj, serializers=serializers)
dumps_rs->>EncodeCtx: tags = serializers.as_ref()
Related issues: None specified. Related PRs: None specified. Suggested labels: documentation, breaking-change, api Suggested reviewers: frenck 🐰
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/features/test_tags.py (2)
221-229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale
tagsin test function name.Docstring/body now describe the
serializerscallback, but the function name still saystest_dumps_tags_callback_bad_return_raises, inconsistent with the sibling renames (test_dumps_serializers_registry_*).✏️ Suggested rename
-def test_dumps_tags_callback_bad_return_raises(): +def test_dumps_serializers_callback_bad_return_raises():
141-263: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing regression test for the breaking-change contract.
Per the PR objectives, passing the old
tags=keyword todumps/dump/async_dumpshould now raiseTypeErrorwith no deprecation alias. None of the updated tests assert this; consider adding one to lock in the intentional break and guard against a future accidental re-introduction of the old kwarg alias.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 83902994-0baa-4d7a-b00a-2e66b91afe5f
📒 Files selected for processing (6)
docs/src/content/docs/guides/tags.mddocs/src/content/docs/reference/api.mdpysrc/yamlrocks/__init__.pypysrc/yamlrocks/__init__.pyisrc/ffi/mod.rstests/features/test_tags.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #222 +/- ##
=======================================
Coverage 92.51% 92.51%
=======================================
Files 41 41
Lines 12720 12720
=======================================
Hits 11768 11768
Misses 952 952 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR renames the dump-side registry keyword argument from tags to serializers across the public API (dumps, dump, async_dump), the type stubs, the documentation, and the tests. The motivation is a pre-1.0 API-surface cleanup: the load-side tags= is keyed by YAML tag string while the dump-side argument was keyed by Python type, so a single shared name implied a symmetry that did not exist. serializers= better describes turning a Python object into an emittable representation and exposes the true parallel (tags/tag_handler on load vs. serializers/default on dump). The internal EncodeCtx.tags field is intentionally left unchanged, and the load-side tags= is unaffected. This is an intentional breaking change with no deprecation alias.
Changes:
- Renamed the dump-side keyword-only argument
tags→serializersin the PyO3dumpssignature and in the pure-Pythondump/async_dumpwrappers and.pyistubs. - Mapped the public
serializersargument onto the existing internalEncodeCtx.tagsfield, leaving encoder internals untouched. - Updated the tags guide, API reference, and
test_tags.pyto useserializers=.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/ffi/mod.rs |
Renames the dumps keyword tags → serializers and forwards it into EncodeCtx.tags; updates the ignored-args comment. |
pysrc/yamlrocks/__init__.py |
Renames the dump/async_dump keyword and its forwarding to dumps/dump. |
pysrc/yamlrocks/__init__.pyi |
Updates the dump-side stub signatures to serializers; load-side tags preserved. |
docs/src/content/docs/reference/api.md |
Documents serializers for dumps/dump. |
docs/src/content/docs/guides/tags.md |
Rewrites the "emit your own types" section and examples to use serializers. |
tests/features/test_tags.py |
Renames dump-side registry tests and updates docstrings/examples to serializers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Breaking change
Yes. The dump-side registry keyword argument is renamed from
tagstoserializersondumps,dump, andasync_dump. Code that passed a type-keyed registry to dump now needs the new name:There is no deprecation alias: the old
tags=on dump now raisesTypeError. This is intentional, done before 1.0 so the name never has to be carried forward. The load-sidetags=(keyed by YAML tag string) is unchanged.Proposed change
On load,
tagsis keyed by the YAML tag string ({"!vec": func}): "when you see this tag, transform the value." On dump, the same-named argument was keyed by Python type ({MyType: func}): "when you see this type, emit it like this." One name covered two different registries keyed by two different things, which read as a symmetry that was not real.Renaming the dump side to
serializersreflects what it actually does (turn a Python object into something emittable) and reveals the true parallel: each side has a registry plus a catch-all.This is part of the pre-1.0 API-surface review: freezing a clearer surface now rather than a confusing one later. The internal
EncodeCtx.tagsfield is left as-is (private, not user-visible).Type of change
Additional information
Checklist
uv run pytestpasses locally. A pull request cannot be merged unless CI is green.uv run ruff check .anduv run ruff format --check .pass.cargo fmt --checkandcargo clippy --all-targets -- -D warningspass.If the change is user-facing:
docs/is added or updated, anddocs/verify_examples.pystill passes.