Skip to content

Add new strategy register_extra_types #645

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

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

makukha
Copy link

@makukha makukha commented May 7, 2025

Contribution to #641.

Converters supported:

  • BaseConverter, Converter, all preconfigured converters

Extra types added:

  • complex
  • uuid.UUID
  • zoneinfo.ZoneInfo

More extra types to be added in this pull request:

  • array.array
  • datetime: date, datetime (not supported by raw BaseConverter and Converter)
  • datetime: time, timedelta
  • decimal.Decimal
  • fraction.Fraction
  • ipaddress: ...
  • numbers: Complex, Real, Rational, Integral
  • numpy: ...
  • pathlib.Path
  • re.Pattern

@makukha
Copy link
Author

makukha commented May 7, 2025

@Tinche sorry for the long delay, had to complete some work. Could you please take a look at this draft? It must have flaws, so please feel free to suggest or make any changes. I added a couple of tests, but probably missed some important test cases. It is a pleasure to work on cattrs codebase :)

@makukha makukha marked this pull request as ready for review May 7, 2025 11:51
@makukha makukha marked this pull request as draft May 9, 2025 07:07
@makukha makukha marked this pull request as ready for review May 9, 2025 09:13
@Tinche
Copy link
Member

Tinche commented May 19, 2025

My baby's sleep regression is kicking my ass so I'm also going to ask for some patience 😇

From what I understand, the api is supposed to be register_extra_types(converter, UUID)? How about we make it register_extra_types(converter, uuid=True)?

Doesn't require an import, gets auto complete, a little more discoverable?

@makukha
Copy link
Author

makukha commented May 20, 2025

Thank you for finding time for this feature! I wish health and strength to you and your family.

For the API design, I was keeping in mind future extensibility with user plugins, so that register_extra_types needs to support an unbounded collection of types. Also, passing types as args explicitly will save user from guessing what specific types are covered by e.g. numpy= kwarg, just in case numpy adds new ones in future, or we will be adding them gradually. With explicit type arguments, error will be raised on import rather than at the run time.

I use cattrs for both config management and sequential processing of datasets. In my use cases, I always have separate file config.py or model.py where I declare the model. I still need to import all the types for annotations, and register_extra_types will be called in this same file. Do you have a different use case?

If you are still confident that kwarg-based approach is better, I have no objections to follow this direction until register_extra_types starts supporting third-party hooks via plugins.

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.

2 participants