[ty] A configuration setting for isinstance narrowing#26360
Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 94.47%. The percentage of expected errors that received a diagnostic held steady at 89.19%. The number of fully passing files held steady at 95/134. |
Memory usage reportSummary
Significant changesClick to expand detailed breakdownprefect
sphinx
trio
flake8
|
|
| /// Configures semantic type inference behavior. | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| #[option_group] | ||
| pub semantics: Option<SemanticsOptions>, |
There was a problem hiding this comment.
I'd have expected the setting to be part of AnalysisOptions, or maybe a sub table in there. What was your reasoning for moving it into a separate section (sorry for commenting while draft, you don't have to answer right away, I just felt worth bringing this up)
There was a problem hiding this comment.
So for, I spent zero time on thinking about the placement of this new option. I just wanted something available for me to play with (and write mdtests that can switch the setting). But thanks for bringing it up, will keep that in mind when we actually choose to add this option!
There was a problem hiding this comment.
Sounds good. I'd have to check on AnalysisOptions. I think one key question will be is whether this option can be at a per file level using overrides (I don't remember whether we allow this for some analysis options or not)
There was a problem hiding this comment.
whether this option can be at a per file level
Oh, right, that is important! Pretty sure we want a semantic change like the one proposed here to apply globally, and not per-file.
There was a problem hiding this comment.
Mypy actually often allows these kinds of settings to be applied per-file, which can be very useful if you're looking to transition your project from very lax settings (when you've only just started using a type checker) to stricter settings, and you want to do so incrementally. A strategy I've used in the past to successfully add mypy --strict to a project is to start with mypy's strict-optional setting being disabled for several files, and then gradually work down the list of files that have it disabled in following PRs.
There was a problem hiding this comment.
My thinking was that this could lead to inconsistencies? Now that I think about it more, I'm not sure how that would look exactly for a setting like this, but I imagine it could be strange that importing symbol A, and then narrowing it with isinstance(...) might lead to a different result than importing A*, a version of A that was narrowed from A in the exporting module (which seems unlikely, but is possible).
| for x in xs: | ||
| reveal_type(x) # revealed: Item | ||
| else: | ||
| reveal_type(xs) # revealed: Item | (list[Item] & ~list[Unknown]) |
There was a problem hiding this comment.
The second union element here is problematic...
There was a problem hiding this comment.
I think we need to "simplify" this to list[Item] & ~Bottom[list[Unknown]] & Any such that we can easily recognize that it has a bottom materialization of Never, which will make it cause much fewer (no?) false positives.
sharkdp
force-pushed
the
david/isinstance-mode
branch
from
ecaf875 to
7d7972e
Compare
3 participants
Summary
Test Plan