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

typing, mostly in AutoWorld.py #476

Merged
merged 1 commit into from
Apr 29, 2022

Conversation

beauxq
Copy link
Collaborator

@beauxq beauxq commented Apr 28, 2022

typing, mostly in AutoWorld.py
includes a bug fix (that was found by static type checking)
in get_filler_item_name

includes a bugfix (that was found by static type checking)
in `get_filler_item_name`
@Berserker66
Copy link
Member

this contains a bit of a stylistic choice of including -> None, which personally I find to be more clutter than useful. Do we want to start adding that everywhere?

@beauxq
Copy link
Collaborator Author

beauxq commented Apr 28, 2022

mypy, which is one of the most popular static type checkers, requires -> None in order for the function definition to be typed.

It is incapable of giving the same quality of analysis without it. So it is not just a stylistic choice.

@beauxq
Copy link
Collaborator Author

beauxq commented Apr 28, 2022

pyright also, with its most strict settings, will not emit that a return type annotation is missing when something besides None is returned.

@Berserker66 Berserker66 merged commit 46d31c3 into ArchipelagoMW:main Apr 29, 2022
@beauxq beauxq deleted the AutoWorld-typing branch April 29, 2022 03:15
@beauxq
Copy link
Collaborator Author

beauxq commented Apr 29, 2022

I agree that it adds a lot of clutter. So I made this feature request on mypy.

python/mypy#12695

But I don't expect it will be implemented any time soon, or maybe ever. I might anticipate the response "Explicit is better than implicit."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants