[docs] Clarify mutating in-place operators for frozensets

[matthiasdiener]

Before this PR, it appears from the documentation that code like

s = frozenset()
s |= {1}
print(s)  # prints frozenset({1})

would not work.

---

📚 Documentation preview 📚: https://cpython-previews--127312.org.readthedocs.build/

[picnixz]

I'm pretty sure I had seen this in a DPO thread or an old issue. As I'm on mobile could you verify it and/or link the relevant thread or issue? thanks in advance.

[matthiasdiener]

This looks like the same issue as #76176 and https://bugs.python.org/issue31995 :-/

Perhaps it is better to add a reference to https://docs.python.org/3/library/operator.html#in-place-operators ?

[picnixz]

I think we should differentiate between the fact that s |= t and s.update(t) are the same for sets but that x |= y for frozen sets really means something like

tmp = x | y
x = tmp

which is not the same as s being modified in-place (no temporary set holding the result). With your new wording I think people could also think that the frozenset is mutated in place but it's not the case. For instance:

def foo(s):
    s |= {1}

s = set()
foo(s)
print(s)  # {1}

f = frozenset()
foo(f)
print(f)  # frozenset()

Since the inplace operators are paired with the inplace methods, I think it's better to keep the current docs.

[matthiasdiener]

Right, but is there perhaps a better way to formulate it? E.g.,

• adding a brief footnote regarding frozensets
• adding a link to https://docs.python.org/3/library/operator.html#in-place-operators

?

As it is right now, the text is confusing, imho. Many people will incorrectly assume that code like the one in my initial message does not work. This is a different situation (again, imho) from the docs for other immutable classes like tuple, where no such strong affirmation is given (e.g., nowhere is it saying that in-place mutation operators aren't supported for tuples).

[skirpichev]

Maybe it's better to use something like "augmented assignments" instead. They actually aren't in-place ops, as frozenset is immutable.

[matthiasdiener]

Thanks, I changed this in a888e53

[skirpichev]

Sorry, not much better for me. As Bénédikt explained above to you, there actually no variants of above methods for frozenset's. E.g. |= for the frozenset does different things than update.

BTW, for set these "variants" actually different internally:

$ ./python -m timeit -unsec -s 's1, s2, s3 = map(set, [[1], [2], [3]])' 's1.update(s2, s3)'
1000000 loops, best of 5: 389 nsec per loop
$ ./python -m timeit -unsec -s 's1, s2, s3 = map(set, [[1], [2], [3]])' 's1 |= s2 | s3'
200000 loops, best of 5: 1.44e+03 nsec per loop

Maybe we should just remove augmented "equivalents"? From docs it's clear what's going on in update() and friends without such "aliases". Also from docs - it's clear that augmented operations are defined, at least if corresponding normal operations are defined. Which is the case both for set and frozenset.

I think that current docs are ok. I'm going to close this, unless Raymond change his opinion. CC @rhettinger, what do you think on just removing augmented "equivalents" for update(), etc?

[matthiasdiener]

Sorry, not much better for me. As Bénédikt explained above to you, there actually no variants of above methods for frozenset's. E.g. |= for the frozenset does different things than update.

I get that, thank you.

Maybe we should just remove augmented "equivalents"? From docs it's clear what's going on in update() and friends without such "aliases".

I'm assuming you mean here to remove the augmented "equivalents" from the docs (and not the actual functionality)? If so, I think this would indeed clarify the docs.

[skirpichev]

remove the augmented "equivalents" from the docs

Yes. To be precise, e.g. update() docs will look as:

   .. method:: update(*others)

      Update the set, adding elements from all others.

and not the actual functionality

I'm not so evil.

I think this would indeed clarify the docs.

I hope so. But it seems, this is a second bugreport for years. So, current docs probably are ok.

[matthiasdiener]

remove the augmented "equivalents" from the docs

Yes. To be precise, e.g. update() docs will look as:

   .. method:: update(*others)

      Update the set, adding elements from all others.

👍

and not the actual functionality

I'm not so evil.

Sure, I just wanted to clarify.

I think this would indeed clarify the docs.

I hope so. But it seems, this is a second bugreport for years. So, current docs probably are ok.

Maybe it is clear, but I suspect it is not. As I indicated in my other comment, I think that many people will read the current docs and simply conclude that |= etc. does not work, without ever trying it or connecting the dots with other parts of the documentation. I think it is not unreasonable to think that from the current docs, frozenset does something to "disable" |=.

[ghost]

All commit authors signed the Contributor License Agreement.

[matthiasdiener]

As a side note, although not many people might have opened an issue/PR regarding this, there are some indicators that this type of misunderstanding might be more widespread, e.g. https://github.com/rindPHI/proxyorderedset/blob/main/src/orderedset/orderedset.py#L298-L299 or https://github.com/corenting/immutabledict/blob/6ad02f98003583416b40dc0816c10d97969d126a/immutabledict/__init__.py#L96-L97

[picnixz]

One way to fix this is to create a dedicated section frozensets. But this could be too verbose.

Let's ask @willingc and @hugovk as docs experts. I would also open a separate issue if you think there is enough confusion in the community.