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

refactor: Use Parameter class directly in type hints instead of _Parameter protocol #3405

Closed
wants to merge 5 commits into from

Conversation

binste
Copy link
Contributor

@binste binste commented Apr 14, 2024

Turns out I was wrong in #3208 (comment) which is nice in this case :) We can use the Parameter class directly in type hints without running into circular imports thanks to typing.TYPE_CHECKING. See https://mypy.readthedocs.io/en/stable/runtime_troubles.html#import-cycles for more details but in summary, type checkers can be much more flexible how they import stuff as they don't run actual code so they can also deal with circular imports.

There is no impact for users except that instead of _Parameter as type hints:

image

they now see the less confusing Parameter:
image

@binste binste requested a review from mattijn June 27, 2024 07:18
@mattijn
Copy link
Contributor

mattijn commented Jun 27, 2024

Is this PR still applicable after merging #3430?

@dangotbanned
Copy link
Member

Is this PR still applicable after merging #3430?

@mattijn I think all of this was covered in that PR, which I could have conveyed better in #3405 (comment)

@binste
Copy link
Contributor Author

binste commented Jun 27, 2024

Good point! I'll remove some comments which are now outdated in #3405

@binste binste closed this Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants