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

Charclass: type signatures and fixed types #83

Merged
merged 12 commits into from
Mar 11, 2023
Merged

Conversation

rwe
Copy link
Contributor

@rwe rwe commented Mar 9, 2023

This fixes the types of the arguments and attributes of Charclass.
The constructor now accepts a general Iterable[str] and exposes a frozenset[str].

It also raises slightly more-specific TypeError/ValueError on invalid entries in the supplied set.

This allows mypy --strict greenery/charclass{_test}.py to pass.

rwe added 12 commits March 8, 2023 19:36
This makes it possible to fix some type errors by distinguishing the
argument types and attribute types.
This fixes some type errors in which it previously could not accept e.g.
a `set`, or which failed to recognize that `Charclass.chars` was always
a `frozenset`.
The `key` iterator was reused for several different loops. Originally
the type is inferred as a Charclass, but the keys to the `escapes`
mapping is `str`.
This allows mypy --strict to pass on greenery/charclass*
@@ -191,9 +191,9 @@ def match_charclass(string: str, i):
pass

# e.g. if seeing "\\t", return "\t"
for key in escapes.keys():
for ekey in escapes.keys():
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is key a reserved name in Python these days?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it was a local name bound to a different type in previous loop expressions in the same function.

@qntm qntm merged commit 73ce68b into qntm:main Mar 11, 2023
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.

None yet

2 participants