Skip to content
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

Add some missing generic type arguments #595

Merged
merged 2 commits into from
Apr 21, 2022

Conversation

RobertCraigie
Copy link
Contributor

@RobertCraigie RobertCraigie commented Apr 12, 2022

This PR adds generic type arguments to type hints in sessions.py, I did not have time to go through the whole codebase unfortunately.

The benefit of adding type arguments is that type checkers like Pyright will complain if you access a variable that has a potentially unknown type, for example I ran into this:

import nox

@nox.session(reuse_venv=True)
def pyright(session: nox.Session) -> None:
    session.env['ENV_VARIABLE'] = '1'
    # Error: Type of "env" is partially unknown
    #            Type of "env" is "dict[Unknown, Unknown]"

If you want to avoid errors like these in the future, I'd highly recommend adding pyright to CI and using --verifytypes.

@henryiii
Copy link
Collaborator

I'd recommend turning up the strictness of mypy, and then this would be enforced. I can take a stab at it.

@FollowTheProcess
Copy link
Collaborator

Hi @RobertCraigie thanks for the PR! This looks great to me

@FollowTheProcess
Copy link
Collaborator

I'm gonna get this one in so @RobertCraigie is recognised as a contributor then planning to merge #596 straight after to enforce going forward, unless anyone from the maintainer team screams "No!" in the meantime 👍🏻

@FollowTheProcess FollowTheProcess merged commit d9e2580 into wntrblm:main Apr 21, 2022
@RobertCraigie RobertCraigie deleted the add-generic-arguments branch April 24, 2022 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants