Core: Utils.py typing #3064
Merged
Core: Utils.py typing #3064
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
`get_fuzzy_results` typing There are places that this is called with a `word_list` that is not a `Sequence`, and it is valid (e.g., `set` or `dict`). To decide the right type, we look at how `word_list` is used: - the parameter to `len` - requires `__len__` - the 2nd parameter to `map` - requires `__iter__` Then we look at https://docs.python.org/3/library/collections.abc.html#collections-abstract-base-classes and ask what is the simplest type that includes both `__len__` and `__iter__`: `Collection` (Python 3.8 requires using the alias in `typing`, instead of `collections.abc`)
github-actions
bot
added
affects: core
Exempt-Medic
added
the
is: refactor/cleanup
There was a problem hiding this comment.
The
Found 40 errors in 1 file (checked 1 source file)
->
Found 33 errors in 1 file (checked 1 source file)
looks good.
I have not double-checked the changes in Collection types, but I trust they are correct.
The only "real" change (as in not just typing) I can see is the assert in open_file, which isn't a great message, but also it isn't worse than getting TypeError: expected str, bytes or os.PathLike object, not NoneType from subprocess. Still suggesting a better message (for people that run from source on not-yet-supported systems).
… anyway... Co-authored-by: black-sliver <[email protected]>
|
Sorry :-( |
jnschurig
pushed a commit
to Tranquilite0/Archipelago-SoulBlazer
that referenced
this pull request
Jun 13, 2024
* Core: Utils.py typing `get_fuzzy_results` typing There are places that this is called with a `word_list` that is not a `Sequence`, and it is valid (e.g., `set` or `dict`). To decide the right type, we look at how `word_list` is used: - the parameter to `len` - requires `__len__` - the 2nd parameter to `map` - requires `__iter__` Then we look at https://docs.python.org/3/library/collections.abc.html#collections-abstract-base-classes and ask what is the simplest type that includes both `__len__` and `__iter__`: `Collection` (Python 3.8 requires using the alias in `typing`, instead of `collections.abc`) * a bit more typing and cleaning * fine, take away my fun for something that no one is ever going to see anyway... Co-authored-by: black-sliver <[email protected]> --------- Co-authored-by: black-sliver <[email protected]>
sflavelle
pushed a commit
to sflavelle/Archipelago-tgc
that referenced
this pull request
Jun 20, 2024
* Core: Utils.py typing `get_fuzzy_results` typing There are places that this is called with a `word_list` that is not a `Sequence`, and it is valid (e.g., `set` or `dict`). To decide the right type, we look at how `word_list` is used: - the parameter to `len` - requires `__len__` - the 2nd parameter to `map` - requires `__iter__` Then we look at https://docs.python.org/3/library/collections.abc.html#collections-abstract-base-classes and ask what is the simplest type that includes both `__len__` and `__iter__`: `Collection` (Python 3.8 requires using the alias in `typing`, instead of `collections.abc`) * a bit more typing and cleaning * fine, take away my fun for something that no one is ever going to see anyway... Co-authored-by: black-sliver <[email protected]> --------- Co-authored-by: black-sliver <[email protected]>
qwint
pushed a commit
to qwint/Archipelago
that referenced
this pull request
Jun 24, 2024
* Core: Utils.py typing `get_fuzzy_results` typing There are places that this is called with a `word_list` that is not a `Sequence`, and it is valid (e.g., `set` or `dict`). To decide the right type, we look at how `word_list` is used: - the parameter to `len` - requires `__len__` - the 2nd parameter to `map` - requires `__iter__` Then we look at https://docs.python.org/3/library/collections.abc.html#collections-abstract-base-classes and ask what is the simplest type that includes both `__len__` and `__iter__`: `Collection` (Python 3.8 requires using the alias in `typing`, instead of `collections.abc`) * a bit more typing and cleaning * fine, take away my fun for something that no one is ever going to see anyway... Co-authored-by: black-sliver <[email protected]> --------- Co-authored-by: black-sliver <[email protected]>
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.
What is this fixing or adding?
get_fuzzy_resultsThere are places that this is called with a
word_listthat is not aSequence, and it is valid (e.g.,setordict).To decide the right type, we look at how
word_listis used:len- requires__len__map- requires__iter__Then we look at https://docs.python.org/3/library/collections.abc.html#collections-abstract-base-classes and ask what is the simplest type that includes both
__len__and__iter__:Collection(Python 3.8 requires using the alias in
typing, instead ofcollections.abc)others
a few similar collection types in other places
I don't see any errors in mypy or pyright from removing the
castin line 104.Line 211, type checking that outside of windows reports an error.
Line 215,
open_commandwould beNoneif someone is not in windows, and macos doesn't have theiropen, and linux doesn't have any of those otheropens.Line 606, mypy and pyright can see that that's
intwithout the annotation. (And with theintthey don't like the redeclaration, because it's declared as a parameter.)How was this tested?
Just type checking and unit tests.