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

frozenset could be "updated" by |= operator, is this intended? #126783

Closed
DanielYang59 opened this issue Nov 13, 2024 · 11 comments
Closed

frozenset could be "updated" by |= operator, is this intended? #126783

DanielYang59 opened this issue Nov 13, 2024 · 11 comments

Comments

@DanielYang59
Copy link

DanielYang59 commented Nov 13, 2024

Bug report

Bug description:

The |= operator behaves unexpectedly when used with frozenset, where it creates a new instance. This can be misleading because it effectively updates it. I would expect this to trigger a TypeError?

fs = frozenset([1, 2, 3])

print(id(fs))  # 4379466816

fs |= {4, 5}

print(fs)  # >>> frozenset({1, 2, 3, 4, 5})

print(id(fs))  # ID changed to 4379462560

CPython versions tested on:

3.9, 3.10, 3.11, 3.12, 3.13

Operating systems tested on:

macOS

@DanielYang59 DanielYang59 added the type-bug An unexpected behavior, bug, or error label Nov 13, 2024
@DanielYang59 DanielYang59 changed the title frozendict could be "updated" by |= operator, is this intended? frozenset could be "updated" by |= operator, is this intended? Nov 13, 2024
@nineteendo
Copy link
Contributor

No, only the reference is updated:

>>> set1 = set2 = frozenset([1, 2, 3])
>>> set2 |= {4, 5} # equivalent to set2 = set2 | {4, 5}
>>> set1
frozenset({1, 2, 3})
>>> set2
frozenset({1, 2, 3, 4, 5})

@skirpichev
Copy link
Member

I don't anything wrong in this, consider:

>>> i = 1234
>>> id(i)
140639472689744
>>> i += 1
>>> i
1235
>>> id(i)
140639472686640

On another hand, docs says: "The following table lists operations available for set that do not apply to immutable instances of frozenset:
update(*others)
set |= other | ...

Update the set, adding elements from all others."

Seems to be a docs issue.

@ZeroIntensity ZeroIntensity added the pending The issue will be closed if no feedback is provided label Nov 13, 2024
@ZeroIntensity
Copy link
Member

If you're interested in mutating immutable objects, please look into footguns such as ctypes 😄

I don't see anything that looks like a bug here.

@nineteendo
Copy link
Contributor

nineteendo commented Nov 13, 2024

https://docs.python.org/3/reference/datamodel.html#object.__ior__:

For instance, if x is an instance of a class with an __iadd__ method, x += y is equivalent to x = x.__iadd__(y) . If __iadd__ does not exist, or if x.__iadd__(y) returns NotImplemented, x.__add__(y) and y.__radd__(x) are considered, as with the evaluation of x + y.

>>> frozenset().__or__
<method-wrapper '__or__' of frozenset object at 0x7f84dbd25900>
>>> frozenset().__ior__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'frozenset' object has no attribute '__ior__'

@DanielYang59
Copy link
Author

Thanks you guys are crazy responsive :)

On another hand, docs says: "The following table lists operations available for set that do not apply to immutable instances of frozenset:

Yes I'm not expecting these operations to work for frozenset (I expect a TypeError), but for some reason they do?

image

@nineteendo
Copy link
Contributor

nineteendo commented Nov 13, 2024

Read my comment above, the same applies to __ior__().

@ZeroIntensity
Copy link
Member

Maybe we could have decided it could raise a TypeError 20 years ago. Now, changing that would break untold amounts of code. Sorry :(

@ZeroIntensity ZeroIntensity closed this as not planned Won't fix, can't repro, duplicate, stale Nov 13, 2024
@ZeroIntensity ZeroIntensity removed type-bug An unexpected behavior, bug, or error pending The issue will be closed if no feedback is provided labels Nov 13, 2024
@DanielYang59
Copy link
Author

Read my comment above, the same applies to ior().

Thanks that's really helpful. I thought |= would internally call __ior__ exactly and it turns out it falls back to x = x.__or__(y). I really appreciate this :)

Maybe we could have decided it could raise a TypeError 20 years ago. Now, changing that would break untold amounts of code. Sorry :(

Understandable :)

@DanielYang59
Copy link
Author

Again you guys are amazingly responsive and helpful, really appreciate that!

@DanielYang59
Copy link
Author

On another hand, docs says: "The following table lists operations available for set that do not apply to immutable instances of frozenset:

Putting the breaking implementation change aside, can I ask for a documentation clarification? As the docs explicitly says "do not apply to immutable instances of frozenset" while they do work?

@ZeroIntensity
Copy link
Member

ZeroIntensity commented Nov 13, 2024

They work, but those methods don't behave as documented on frozenset as they do set.

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

4 participants