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

Adding a make_dict_unstructure_fn breaks omit_if_default=True #238

Open
MicaelJarniac opened this issue Mar 24, 2022 · 2 comments
Open

Adding a make_dict_unstructure_fn breaks omit_if_default=True #238

MicaelJarniac opened this issue Mar 24, 2022 · 2 comments

Comments

@MicaelJarniac
Copy link
Contributor

  • cattrs version: 1.10.0
  • attrs version: 21.4.0
  • Python version: 3.8.12
  • Operating System: Manjaro Linux

Description

I'm yet to get a minimal working example, but it's something like so:

@attrs.define
class Whatever:
    foo: str = "foo"
    bar: str = "bar"
    thing: str = "thing"
    u_thing: str = "_thing"

whatever = Whatever(bar="baz")


c = cattrs.GenConverter(omit_if_default=True)

c.unstructure(whatever)  # Does omit if default!

unstruct_hook = cattr.gen.make_dict_unstructure_fn(
    Whatever,
    c,
    **{"u_thing": cattr.override(rename="_thing")}
)
c.register_unstructure_hook(Whatever, unstruct_hook)

c.unstructure(whatever)  # Does NOT omit if default :c

I do a lot of other things to the converters, but from my testing, this is what's breaking it.

I need those extra hooks because I need both thing and _thing to show up on the output, and attrs doesn't seem to allow that, so I need to rename it on the fly.

@MicaelJarniac
Copy link
Contributor Author

Sort of related, but not quite: python-attrs/attrs#391

@Tinche
Copy link
Member

Tinche commented Mar 24, 2022

So when you pass omit_if_default=True to the GenConverter, it'll use it when it calls make_dict_unstructure_fn for you. But if you're calling it yourself, you need to pass it in yourself too. So do:

unstruct_hook = cattr.gen.make_dict_unstructure_fn(
    Whatever,
    c,
    u_thing=cattr.override(rename="_thing"), _cattrs_omit_if_default=True
)
c.register_unstructure_hook(Whatever, unstruct_hook)

The parameter name is a little mangled to not clash with a possible attribute of that name.

We could make it so that the converter behavior is used if the flag is not set explicitly. If you feel like putting together a PR for this, I'd be happy to merge it in ;)

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

No branches or pull requests

2 participants