python / typeshed

Collection of library stubs for Python, with static types
Other
4.33k stars 1.73k forks source link

Incorrect types for `keyword.kwlist` and `keyword.softkwlist` #9679

Closed Eclips4 closed 1 year ago

Eclips4 commented 1 year ago

For some reason, kwlist and softkwlist typed as Sequence[str] instead of list[str] Corresponding to Lib/keyword.py, it's pure lists.

TeamSpen210 commented 1 year ago

This is probably intentional - these attributes are supposed to be constants, and really shouldn't be modified. Annotating them as Sequence will make the type checker flag any modification attempts. Could do with having a comment to mention that, and also making them Final[Sequence[str]].

Eclips4 commented 1 year ago

Thank you! I think, it's good idea to making them Final[list[str]], instead of Final[Sequence[str]]. Why? This test raises an Error: Expected type 'list', got 'Sequence[str]' instead Because of unittest.TestCase.assertListEqual, which accepts two lists. And according to this test, changing value of kwlist breaks nothing. What you think about this?

TeamSpen210 commented 1 year ago

Python's test suite shouldn't be an example of valid type-checkable code. In that case perhaps the test should be loosened to assertSequenceEqual(), making the fact that it's a list an implementation detail. Or treat the test as confirming the internal details match also. In general test suites aren't really good for type checking, since they may look at internals, do dynamic things like monkeypatching and deliberately run invalid code to confirm it errors. In CPython's case, it's also ensuring that you don't get fatal errors even for the most absurd code. For the second example, it has a comment saying that they're testing an implementation detail to ensure it doesn't change, in case someone is accidentally relying on it.

Eclips4 commented 1 year ago

Okay. I will change it to Final[Sequence[str]]. Thx for advices!

AlexWaygood commented 1 year ago

I agree with @TeamSpen210 — these are documented as "sequences" rather than "lists": https://docs.python.org/3/library/keyword.html#keyword.kwlist

Trying to alter them at runtime would likely indicate buggy assumptions in a Python program. Better to keep them typed as if they're immutable.