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

Add more correct type annotations for tomllib #10031

Closed
wants to merge 4 commits into from

Conversation

wchistow
Copy link

I've added more correct type annotations for tomllib module, because it was Any, but there is a table of types in documentation.

@AlexWaygood
Copy link
Member

Note to reviewers: mypy_primer won't be much use on this PR, since we run mypy_primer on Python 3.10, and tomllib is new in Python 3.11.

@github-actions

This comment has been minimized.

@AlexWaygood
Copy link
Member

Note to reviewers: mypy_primer won't be much use on this PR, since we run mypy_primer on Python 3.10, and tomllib is new in Python 3.11.

We now run mypy_primer on 3.11 following #9336. I've merged main into this branch so that we get a mypy_primer re-run.

stdlib/tomllib.pyi Outdated Show resolved Hide resolved
stdlib/tomllib.pyi Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

pytest (https://github.com/pytest-dev/pytest)
+ src/_pytest/config/findpaths.py:79: error: Item "int" of "Union[int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "get"  [union-attr]
+ src/_pytest/config/findpaths.py:79: error: Item "float" of "Union[int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "get"  [union-attr]
+ src/_pytest/config/findpaths.py:79: error: Item "str" of "Union[int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "get"  [union-attr]
+ src/_pytest/config/findpaths.py:79: error: Item "List[_TOMLValue]" of "Union[int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "get"  [union-attr]
+ src/_pytest/config/findpaths.py:79: error: Item "date" of "Union[int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "get"  [union-attr]
+ src/_pytest/config/findpaths.py:79: error: Item "time" of "Union[int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "get"  [union-attr]
+ src/_pytest/config/findpaths.py:79: error: Item "int" of "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "get"  [union-attr]
+ src/_pytest/config/findpaths.py:79: error: Item "float" of "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "get"  [union-attr]
+ src/_pytest/config/findpaths.py:79: error: Item "str" of "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "get"  [union-attr]
+ src/_pytest/config/findpaths.py:79: error: Item "List[_TOMLValue]" of "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "get"  [union-attr]
+ src/_pytest/config/findpaths.py:79: error: Item "date" of "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "get"  [union-attr]
+ src/_pytest/config/findpaths.py:79: error: Item "time" of "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "get"  [union-attr]
+ src/_pytest/config/findpaths.py:87: error: Key expression in dictionary comprehension has incompatible type "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]"; expected type "str"  [misc]
+ src/_pytest/config/findpaths.py:87: error: Item "int" of "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "items"  [union-attr]
+ src/_pytest/config/findpaths.py:87: error: Item "float" of "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "items"  [union-attr]
+ src/_pytest/config/findpaths.py:87: error: Item "str" of "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "items"  [union-attr]
+ src/_pytest/config/findpaths.py:87: error: Item "List[_TOMLValue]" of "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "items"  [union-attr]
+ src/_pytest/config/findpaths.py:87: error: Item "date" of "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "items"  [union-attr]
+ src/_pytest/config/findpaths.py:87: error: Item "time" of "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "items"  [union-attr]

pylint (https://github.com/pycqa/pylint)
+ pylint/config/find_default_config_files.py:29: error: Unsupported right operand type for in ("Union[int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]")  [operator]
+ pylint/config/config_file_parser.py:71: error: Value of type "Union[int, float, str, bool, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], datetime, date, time]" is not indexable  [index]
+ pylint/config/config_file_parser.py:71: error: No overload variant of "__getitem__" of "list" matches argument type "str"  [call-overload]
+ pylint/config/config_file_parser.py:71: note: Possible overload variants:
+ pylint/config/config_file_parser.py:71: note:     def __getitem__(self, SupportsIndex, /) -> _TOMLValue
+ pylint/config/config_file_parser.py:71: note:     def __getitem__(self, slice, /) -> List[_TOMLValue]
+ pylint/config/config_file_parser.py:71: error: Invalid index type "str" for "Union[int, float, str, bool, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], datetime, date, time]"; expected type "Union[SupportsIndex, slice]"  [index]
+ pylint/config/config_file_parser.py:77: error: Item "str" of "Union[Any, str, int, float, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "items"  [union-attr]
+ pylint/config/config_file_parser.py:77: error: Item "int" of "Union[Any, str, int, float, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "items"  [union-attr]
+ pylint/config/config_file_parser.py:77: error: Item "float" of "Union[Any, str, int, float, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "items"  [union-attr]
+ pylint/config/config_file_parser.py:77: error: Item "List[_TOMLValue]" of "Union[Any, str, int, float, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "items"  [union-attr]
+ pylint/config/config_file_parser.py:77: error: Item "date" of "Union[Any, str, int, float, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "items"  [union-attr]
+ pylint/config/config_file_parser.py:77: error: Item "time" of "Union[Any, str, int, float, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" has no attribute "items"  [union-attr]
+ pylint/config/config_file_parser.py:81: error: Invalid index type "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" for "Dict[str, str]"; expected type "str"  [index]
+ pylint/config/config_file_parser.py:85: error: Invalid index type "Union[Any, int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" for "Dict[str, str]"; expected type "str"  [index]

poetry (https://github.com/python-poetry/poetry)
+ src/poetry/packages/locker.py:87: error: Unsupported right operand type for in ("Union[int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]")  [operator]
+ src/poetry/packages/locker.py:88: error: Value of type "Union[int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]" is not indexable  [index]
+ src/poetry/packages/locker.py:88: error: No overload variant of "__getitem__" of "list" matches argument type "str"  [call-overload]
+ src/poetry/packages/locker.py:88: note: Possible overload variants:
+ src/poetry/packages/locker.py:88: note:     def __getitem__(self, SupportsIndex, /) -> _TOMLValue
+ src/poetry/packages/locker.py:88: note:     def __getitem__(self, slice, /) -> List[_TOMLValue]
+ src/poetry/packages/locker.py:88: error: Invalid index type "str" for "Union[int, float, str, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], date, time]"; expected type "Union[SupportsIndex, slice]"  [index]
+ src/poetry/packages/locker.py:318: error: Item "int" of "Union[int, float, str, bool, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], datetime, date, time]" has no attribute "get"  [union-attr]
+ src/poetry/packages/locker.py:318: error: Item "float" of "Union[int, float, str, bool, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], datetime, date, time]" has no attribute "get"  [union-attr]
+ src/poetry/packages/locker.py:318: error: Item "str" of "Union[int, float, str, bool, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], datetime, date, time]" has no attribute "get"  [union-attr]
+ src/poetry/packages/locker.py:318: error: Item "bool" of "Union[int, float, str, bool, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], datetime, date, time]" has no attribute "get"  [union-attr]
+ src/poetry/packages/locker.py:318: error: Item "List[_TOMLValue]" of "Union[int, float, str, bool, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], datetime, date, time]" has no attribute "get"  [union-attr]
+ src/poetry/packages/locker.py:318: error: Item "datetime" of "Union[int, float, str, bool, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], datetime, date, time]" has no attribute "get"  [union-attr]
+ src/poetry/packages/locker.py:318: error: Item "date" of "Union[int, float, str, bool, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], datetime, date, time]" has no attribute "get"  [union-attr]
+ src/poetry/packages/locker.py:318: error: Item "time" of "Union[int, float, str, bool, List[_TOMLValue], Dict[_TOMLValue, _TOMLValue], datetime, date, time]" has no attribute "get"  [union-attr]

@AlexWaygood
Copy link
Member

AlexWaygood commented Apr 11, 2023

Just taking the first pytest error in the mypy_primer diff as an example, they're doing this:

config = tomllib.loads(toml_text)
result = config.get("tool", {}).get("pytest", {}).get("ini_options", None)

https://github.com/pytest-dev/pytest/blob/22524046cff84c66f128da9e3cdb993082445c75/src/_pytest/config/findpaths.py#L74-L79

That seems like pretty idiomatic use of tomllib to me, but with this PR, mypy would complain loudly about it.

The stdlib tomllib is also exactly the same code as the third-party library tomli, which uses dict[str, Any] as its return types for these functions: https://github.com/hukkin/tomli/blob/1bd3345f97cba795d7e6075956815c0a52151ed0/src/tomli/_parser.py#L57-L132. I'm not sure it makes sense for typeshed to deviate from tomli's annotations here, given most libraries will be only using tomllib on 3.11+, and will be falling back to tomli on Python <=3.10.

Lastly, even if we decided that this PR is worth the disruption, it's not currently correct. The docs state:

parse_float will be called with the string of every TOML float to be decoded. By default, this is equivalent to float(num_str). This can be used to use another datatype or parser for TOML floats (e.g. decimal.Decimal).

That means that if the user provides a value to the parse_float parameter, you could have different types being returned. We could possibly represent this by using overloads and making _TOMLValue a generic-recursive type alias, but this would make the type annotation very complicated, and increase the risk of type checkers emitting false positives for users of tomllib.

@AlexWaygood
Copy link
Member

Cc. @hauntsaninja & @hukkin for tomllib.

@srittau
Copy link
Collaborator

srittau commented Apr 11, 2023

I agree that the current annotation of dict[str, Any] is probably the best we can do for now, until we get permissive unions (python/typing#566).

@AlexWaygood
Copy link
Member

Let's close this for now, then. Happy to reopen if @hauntsaninja or @hukkin disagree strongly with this decision :)

@wchistow wchistow deleted the issue-10025 branch July 9, 2023 08:37
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.

3 participants