Skip to content
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

Allow overriding unstruct_collection_overrides in preconf make_converter #350

Closed
ericbn opened this issue Apr 10, 2023 · 2 comments · Fixed by #353
Closed

Allow overriding unstruct_collection_overrides in preconf make_converter #350

ericbn opened this issue Apr 10, 2023 · 2 comments · Fixed by #353

Comments

@ericbn
Copy link
Contributor

ericbn commented Apr 10, 2023

  • cattrs version: 22.2.0
  • Python version: 3.9.16
  • Operating System: macOS

Description

What I was trying to do:

from collections.abc import Set

from cattrs.preconf.json import make_converter as json_make_converter

json_converter = json_make_converter(unstruct_collection_overrides={Set: sorted})

This didn't work because cattrs.preconf.json.make_converter overrides the kwargs:

kwargs["unstruct_collection_overrides"] = {
**kwargs.get("unstruct_collection_overrides", {}),
AbstractSet: list,
Counter: dict,
}

Then I tried:

from collections.abc import Set

from cattrs.preconf.json import make_converter as json_make_converter

json_converter = json_make_converter().copy(unstruct_collection_overrides={Set: sorted})

But later in the code I call json_converter.dumps and mypy complained error: "Converter" has no attribute "dumps" [attr-defined] because copy returns a Converter.

What I Did

from collections.abc import Set

from cattrs.preconf.json import JsonConverter
from cattrs.preconf.json import configure_converter as json_configure_converter

json_converter = JsonConverter(unstruct_collection_overrides={Set: sorted})
json_configure_converter(json_converter)

Maybe the two forms that I've tried above should also be allowed?

@Tinche
Copy link
Member

Tinche commented Apr 11, 2023

Howdy,

I think it's an oversight.

Could you change the lines:

kwargs["unstruct_collection_overrides"] = {
        **kwargs.get("unstruct_collection_overrides", {}),
        AbstractSet: list,
        Counter: dict,
    }

to

kwargs["unstruct_collection_overrides"] = {
        AbstractSet: list,
        Counter: dict,
        **kwargs.get("unstruct_collection_overrides", {}),
    }

, add a small test and submit a PR?

@ericbn
Copy link
Contributor Author

ericbn commented Apr 11, 2023

Hi. I'll work on a PR. Thanks for the quick response!

ericbn added a commit to ericbn/cattrs that referenced this issue Apr 12, 2023
ericbn added a commit to ericbn/cattrs that referenced this issue Apr 12, 2023
Tinche pushed a commit that referenced this issue Apr 14, 2023
* Allow overriding unstruct_collection_overrides

Fixes #350

* Update HISTORY.md
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 a pull request may close this issue.

2 participants