Skip to content

Fix isset() to cover dict.items() and dict.keys() in set comparisons#14576

Merged
bluetech merged 4 commits into
pytest-dev:mainfrom
croc100:fix/isset-dict-items-view
Jun 25, 2026
Merged

Fix isset() to cover dict.items() and dict.keys() in set comparisons#14576
bluetech merged 4 commits into
pytest-dev:mainfrom
croc100:fix/isset-dict-items-view

Conversation

@croc100

@croc100 croc100 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Closes #12860.

isset() was checking isinstance(x, set | frozenset), so dict.items() and dict.keys() — which both implement collections.abc.Set — never triggered the set comparison path. The result was a raw repr diff with no useful information:

E  AssertionError: assert dict_items([('a', 1), ('b', 2)]) >= dict_items([('a', 1), ('b', 2), ('c', 3)])
E   +  where dict_items(...) = {'a': 1, 'b': 2}.items()
E   +  and   dict_items(...) = {'a': 1, 'b': 2, 'c': 3}.items()

After this fix:

E  AssertionError: assert dict_items([('a', 1), ('b', 2)]) >= dict_items([('a', 1), ('b', 2), ('c', 3)])
E    Extra items in the right set:
E    ('c', 3)

The fix is to widen isset() to isinstance(x, collections.abc.Set). This covers set, frozenset, dict.items(), and dict.keys() — all of which support - and the comparison operators that the set assertion path relies on. The existing _compare_set.py functions already type their parameters as AbstractSet, so no changes there were needed.

isset() was limited to set/frozenset, so assert expressions like
dict.items() >= other.items() fell through without any diff output.

dict.items() and dict.keys() both implement collections.abc.Set, so
widening the check to isinstance(x, AbstractSet) is enough.

Fixes pytest-dev#12860
@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Jun 8, 2026

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@croc100

croc100 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Hi — just wanted to check in since it's been about a week since @Pierre-Sassoulas left an LGTM.

I'm fairly new to contributing to larger open source projects, so I want to make sure I haven't missed anything on my end. Is there anything I should fix, rebase, or add before this can move forward? Happy to make any changes needed.

Thank you for your time, and apologies if I'm jumping in too early — I wasn't sure what the typical timeline looks like here.

@Pierre-Sassoulas

Copy link
Copy Markdown
Member

Hey @croc100 , just letting other maintainers the opportunity to review if they wish, nothing to do on your part.

@bluetech bluetech merged commit 9db6646 into pytest-dev:main Jun 25, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided (automation) changelog entry is part of PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Better reporting for "dict subset" comparison

3 participants