You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Underdocumented and partially untested behavior of set-like dict views #96408
Unlike sets, set-like dict view objects behave identical with both operator and non-operator versions of set operations.
In case of sets only non-operator versions accept any iterable as an argument, and one has to explicitly convert iterable to set to use operator on it. That's documented and tested.
The dict view docs intended cover these behaviors in the final sentence:
For set-like views, all of the operations defined for the abstract base class collections.abc.Set are available (for example, ==, <, or ^).
However, if you didn't already know the differences between concrete set API and the abstract Set API, I could see how this particular point is easily missed, so perhaps an extra sentence highlighting the difference would be warranted:
Those operations difference from the corresponding set operations by supporting any iterable as a input.
As you say, the tests need to be beefed-up. They test registration with KeysView, ValuesView, and ItemsView but do not test compliance with those APIs.
I've just reconsidered having an example and now think it should be taken out. It was correct to document that iterables were supported but we don't want an example to encourage it. It's too easy to fall into keys | 'abc' instead of keys | ['abc'].
Aren't we heading into a more general discussion now (where my opinion would be: if an API is exposed, it should be accessible and documented; if, on the other hand, it's considered harmful, it shouldn't be exposed...)?
Just for the background of this ticket (it's a real life scenario, so please bare with me): we had a bit of code in our codebase where we used a sorted(a_dict.keys()) bit; then we discovered that those keys don't cover the data we need, and we wanted to extend it like this sorted(a_dict.keys() + ['missing', 'data']), which obviously failed. Then we tried this (it was an intuitive call): sorted(a_dict.keys() | ['missing', 'data']) which worked well and seemed somewhat elegant, but we realized that we can't find neither docs for this behavior, nor examples. Thus we went back to sorted(list(a_dict.keys()) + ['missing', 'data']), which -- it's a matter of personal opinion, I guess -- is not so elegant, but we were doc-covered at least. TL;DR: discovering an elegant and concise API that suited our need but was hard enough to find in the documention was quite confusing and upsetting :)
Activity
rhettinger commentedon Aug 30, 2022
The dict view docs intended cover these behaviors in the final sentence:
However, if you didn't already know the differences between concrete
setAPI and the abstractSet API, I could see how this particular point is easily missed, so perhaps an extra sentence highlighting the difference would be warranted:As you say, the tests need to be beefed-up. They test registration with
KeysView,ValuesView, andItemsViewbut do not test compliance with those APIs.Would you like to submit a PR?
pythongh-96408: Test set operation on items dict view.
pythongh-96408: Document difference between set-like view and sets.
pythongh-96408: Document difference between set-like view and sets.
gh-96408: Test set operation on items dict view. (GH-96438)
pythongh-96408: Document difference between set-like view and sets.
gh-96408: Document difference between set-like view and sets. (GH-96439)
rhettinger commentedon Aug 31, 2022
I've just reconsidered having an example and now think it should be taken out. It was correct to document that iterables were supported but we don't want an example to encourage it. It's too easy to fall into
keys | 'abc'instead ofkeys | ['abc'].filiplajszczak commentedon Aug 31, 2022
If I understand correctly, it (possibility of easy error) was the reason to make it impossible for sets.
Maybe it could be possible, to replace keys example with items one with tuple of tuples instead of list of strings as the other operand.
caseneuve commentedon Sep 1, 2022
Aren't we heading into a more general discussion now (where my opinion would be: if an API is exposed, it should be accessible and documented; if, on the other hand, it's considered harmful, it shouldn't be exposed...)?
Just for the background of this ticket (it's a real life scenario, so please bare with me): we had a bit of code in our codebase where we used a
sorted(a_dict.keys())bit; then we discovered that those keys don't cover the data we need, and we wanted to extend it like thissorted(a_dict.keys() + ['missing', 'data']), which obviously failed. Then we tried this (it was an intuitive call):sorted(a_dict.keys() | ['missing', 'data'])which worked well and seemed somewhat elegant, but we realized that we can't find neither docs for this behavior, nor examples. Thus we went back tosorted(list(a_dict.keys()) + ['missing', 'data']), which -- it's a matter of personal opinion, I guess -- is not so elegant, but we were doc-covered at least. TL;DR: discovering an elegant and concise API that suited our need but was hard enough to find in the documention was quite confusing and upsetting :)pochmann commentedon Sep 2, 2022
@caseneuve Just alternatives for your example: