lint: narrow types to ensure correct usage #17519
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Since `object_session(...)` can return `None`, we need to help mypy have confidence that the session is indeed there. One way to narrow the type is to assert that it's not `None`, and therefore should not alert about `union-attr` problems. Includes a couple of renames, and variable extraction where relevant. Signed-off-by: Mike Fiedler <[email protected]>
Refs: https://mypy.readthedocs.io/en/stable/type_narrowing.html Signed-off-by: Mike Fiedler <[email protected]>
Signed-off-by: Mike Fiedler <[email protected]>
Signed-off-by: Mike Fiedler <[email protected]>
miketheman
added
testing
di
reviewed
Jan 30, 2025
| @@ -31,6 +31,7 @@ def run_migrations_offline(): | |||
| script output. | |||
| """ | |||
| options = context.config.get_section(context.config.config_ini_section) | |||
| assert options is not None # type narrowing | |||
There was a problem hiding this comment.
We should probably better handle the case where options could be None here (raise an exception?) instead of assuming it will always be non-None.
| @@ -237,6 +237,7 @@ def has_primary_verified_email(self): | |||
| @property | |||
| def recent_events(self): | |||
| session = orm.object_session(self) | |||
| assert session is not None | |||
There was a problem hiding this comment.
This (and the many other assert session is not Nones makes me think we probably need a helper function that is responsible for narrowing the type and explicitly handling the case where the session might be None rather than assuming it always is and throwing an AssertionError if it's not. AFAICT, this probably should just raise our own exception with more details.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TL,DR: use type narrowing on objects of ambiguous nature.
These commits push us closer to being able to run
mypywith--check-untyped-defs, we're not fully there yet, but let's get these out the door.See each commit message for specifics on why the particular action was taken.
With these changes in place, we're down to: