-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Set difference strictly #3886
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
Set difference strictly #3886
Conversation
2f03dc7
to
22ffd19
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just ran this against our internal code base and it flagged no problems. This is still a bit risky, so I'd prefer one other review. (@JelleZijlstra @ilevkivskyi @JukkaL)
This can generate false positives, since the set item types arguably only need to be overlapping, not compatible. I wouldn't be surprised if this would generate issues with our internal codebases (but I haven't verified yet). I'll run this against our internal codebases next week (if I remember to :-). |
@JukkaL Did you remember? :) |
I've been too distracted. I'll try to find some time to try this out later this week. |
I am going ahead and merge this. If this causes too many problems down the line, we need to revert it. |
@hauntsaninja ran mypy-primer on recent master (python/mypy#9290) and one new false positive seems to come from this change: |
@JelleZijlstra Is that really a false positive, though? While we could add a special exception for |
Honestly the code (which is at https://github.com/home-assistant/core/blob/dev/homeassistant/helpers/device_registry.py#L568) doesn't seem particularly problematic to me. It's a different story if you have completely non-overlapping types like |
Then again, maybe this case is rare enough that the change is still worth it; we'll have to see how often this comes up in practice. |
close #1840
I think sub operator too.
Because "frozenset" also strictly compare.
It makes sense for consistency.