| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
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)
Sorry, something went wrong.
|
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 :-). |
Sorry, something went wrong.
|
I've been too distracted. I'll try to find some time to try this out later this week. |
Sorry, something went wrong.
|
I am going ahead and merge this. If this causes too many problems down the line, we need to revert it. |
Sorry, something went wrong.
|
@hauntsaninja ran mypy-primer on recent master (python/mypy#9290) and one new false positive seems to come from this change: |
Sorry, something went wrong.
|
@JelleZijlstra Is that really a false positive, though? While we could add a special exception for None, the right-hand side set containing a None hints at a potential problem to me. Of course, it will work at runtime, but so will Set[str] - Set[int]. |
Sorry, something went wrong.
|
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 str and int, but Set[str] - Set[Optional[str]] doesn't seem like an unreasonable thing to do. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
close #1840
I think sub operator too.
Because "frozenset" also strictly compare.
It makes sense for consistency.