Skip to content

Do not untructure the Singleton attrs.NOTHING at all (identity) instead of to 1 (int) (attrs >= 22.2 problem)#667

Closed
raabf wants to merge 2 commits into
python-attrs:mainfrom
raabf:fix-attrs-nothing-enum-unstructure-not-int
Closed

Do not untructure the Singleton attrs.NOTHING at all (identity) instead of to 1 (int) (attrs >= 22.2 problem)#667
raabf wants to merge 2 commits into
python-attrs:mainfrom
raabf:fix-attrs-nothing-enum-unstructure-not-int

Conversation

@raabf
Copy link
Copy Markdown
Contributor

@raabf raabf commented Jul 3, 2025

Since attrs >= 22.2 the attrs.NOTHING is not an object any more but implemented by an enum.Enum, see here.

That causes the follow up-problem, since cattrs supports unstructuring enum.Enum, it will unstructure it to 1 (int).

>>> import cattrs
>>> import attrs
>>> conv = cattrs.Converter()
>>> conv.unstructure(attrs.NOTHING)
1

but this is unintended as attrs.NOTHING should not represent an int but a special state in an attrs Attribute.

That leads to two problems.

First, attrs do set the value of an class attribute to attrs.NOTHING in some faulty states. When you now unstructure such a field with cattrs, it suddenly becomes an 1.

Second, when you create an attrs Attribute (with attrs.field or type annotation) with no default the resulting object is an attrs.Attribute which attrs.Attribute.default value is set to attrs.NOTHING. So in that context, attrs.NOTHING means the attribute does not have any default value.

However, I have now situations where I compare a value of an attribute with its default (i.e. comparing it with attrs.Attribute.default). And if the attribute is an int-field and by chance was set to 1 it is now suddenly equal to its default value although it is defined to have no default at all — which leads to some errors in my case.

Due to attrs.NOTHING’s special meaning of being a faulty state or that no default value is set, I think it is better to not unstructure it (identity as I do in this PR) instead of unstructure it to the (unexpected) int-value of 1.

@raabf raabf force-pushed the fix-attrs-nothing-enum-unstructure-not-int branch from 1efa2f7 to 3867a46 Compare July 3, 2025 17:34
The test could randomly fail as on upickling a python-set creates a new
instance, and then randomly the order could be different then before and
hence the test fails then. As this is not important for the test we
simplify it to a one-element set.
@raabf
Copy link
Copy Markdown
Contributor Author

raabf commented Jul 3, 2025

I did a small update to the #666 unit tests and just use one element in the set of ForbiddenExtraKeysError.

The test could randomly fail as on unpickling a python-set creates a new
instance, and then randomly the order could be different then before and
hence the test fails then. As this is not important for the test we
simplify it to a one-element set.

@Tinche
Copy link
Copy Markdown
Member

Tinche commented Jul 4, 2025

I don't really think this belongs in cattrs, especially since it's very easily overridden by users and I've never heard anyone else run into this issue.

Which states cause attrs to assign NOTHING to instance variables? I'm curious since I maintain attrs as well.

However, I have now situations where I compare a value of an attribute with its default (i.e. comparing it with attrs.Attribute.default). And if the attribute is an int-field and by chance was set to 1 it is now suddenly equal to its default value although it is defined to have no default at all — which leads to some errors in my case.

Yeah, once we have the singleton PEP we can handle this more gracefully. In the meantime, you can probably have a code path that checks if default is NOTHING to handle this case.

@raabf
Copy link
Copy Markdown
Contributor Author

raabf commented Jul 4, 2025

I don't really think this belongs in cattrs,

Well there might be a better representation for attrs.NOTHING in the attrs library in the future which does not have this problem any more, and should be then removed again, but in the meantime this MR might be an acceptable workaround.

By the way this MR restores the behaviour (from an abstract user perspective) of pre-attrs-22.2.

To underline what I mean, I expect that a comparison of an attributes-field with its default value only returns True when it is indeed equal with its default, but this is not the case:

@attrs.define()
class MyA:

    a1: int
    a2: bool
    a3: int = 42


conv = cattrs.Converter()
print('NOTHING is `1`:', conv.unstructure(attrs.NOTHING))

mya1 = MyA(a1=1, a2=True)

mya1_dict = conv.unstructure(mya1)
print('mya1_dict', mya1_dict)

attribs = attrs.fields_dict(MyA)

print('')
print("a3 structured, should be True:", mya1.a3 == attribs['a3'].default)
print("a3 unstructured, should be True:", mya1_dict['a3'] == conv.unstructure(attribs['a3'].default))

print('')
print("a1 structured, should be False as there is no default:", mya1.a1 == attribs['a1'].default)
print(
    "a1 unstructured, should be False as there is no default:",
    mya1_dict['a1'] == conv.unstructure(attribs['a1'].default),  # Default is NOTHING here
)

print('')
print("a2 structured, should be False as there is no default:", mya1.a1 == attribs['a2'].default)
print(
    "a2 unstructured, should be False as there is no default:",
    mya1_dict['a2'] == conv.unstructure(attribs['a2'].default),  # Default is NOTHING here
)
NOTHING is `1`: 1
mya1_dict {'a1': 1, 'a2': True, 'a3': 42}

a3 structured, should be True: True
a3 unstructured, should be True: True

a1 structured, should be False as there is no default: False
a1 unstructured, should be False as there is no default: True

a2 structured, should be False as there is no default: False
a2 unstructured, should be False as there is no default: True

especially since it's very easily overridden by users

Sorry what is overwritten?

and I've never heard anyone else run into this issue.

Then I am the first! 😄

Which states cause attrs to assign NOTHING to instance variables? I'm curious since I maintain attrs as well.

I looked through my Unit-Tests and I fear I did not found an example which documented this behaviour. I remember that I stumbled over this many years ago, and I kept in mind that I should be prepared for this case. If you are confused, then it was probably not an intended behaviour and was some wired edge case and might be gone already over the years ...

However, I have now situations where I compare a value of an attribute with its default (i.e. comparing it with attrs.Attribute.default). And if the attribute is an int-field and by chance was set to 1 it is now suddenly equal to its default value although it is defined to have no default at all — which leads to some errors in my case.

Yeah, once we have the singleton PEP we can handle this more gracefully.

Sounds great to have a solution for that!

In the meantime, you can probably have a code path that checks if default is NOTHING to handle this case.

Don’t worry did already before I opened this MR as it is just an if-clause — still think this would improve the behaviour of cattrs as stated above.

@Tinche
Copy link
Copy Markdown
Member

Tinche commented Jul 4, 2025

To underline what I mean, I expect that a comparison of an attributes-field with its default value only returns True when it is indeed equal with its default, but this is not the case:

Ah, but this is simply not how it's designed or implemented at the moment. The logical type of Attribute[T].default is, essentially, Literal[NOTHING] | T. So if you want to check whether mya1_dict["a1"] is equal to the default, the proper way of doing this would be:

print(
    "a1 unstructured, should be False as there is no default:",
    (
        (mya1_dict["a1"] == conv.unstructure(attribs["a1"].default))
        if attribs["a1"].default is not attrs.NOTHING
        else False
    ),
)

and even that logic can be improved a little bit:

print(
    "a1 unstructured, should be False as there is no default:",
    (
        (
            mya1_dict["a1"]
            == conv.unstructure(
                attribs["a1"].default, unstructure_as=attribs["a1"].type
            )
        )
        if attribs["a1"].default is not attrs.NOTHING
        else False
    ),
)

In any case, this seems too niche to include in cattrs. I need to weigh contributions against the maintenance burden to maintainers and the complexity burden to users, and this doesn't meet the bar, unfortunately. Thanks in any case!

@Tinche Tinche closed this Jul 4, 2025
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