Skip to content

improve type annotations in 'docutils.statemachine' - #11469

Merged
srittau merged 26 commits into
python:mainfrom
danieleades:docutils.statemachine
Mar 13, 2024
Merged

srittau merged 26 commits into
python:mainfrom
danieleades:docutils.statemachine

Conversation

@danieleades

@danieleades danieleades commented Feb 26, 2024 •

Copy link
Copy Markdown
Contributor

adds type annotations for docutils.statemachine.

I'm not super confident about the use of the generic _Context for the state machine since the only code examples I can find using this class are in the sphinx repo, and there no context is used (ever).

@danieleades
danieleades force-pushed the docutils.statemachine branch 5 times, most recently from 55abe87 to 0212302 Compare February 26, 2024 20:06
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@danieleades
danieleades force-pushed the docutils.statemachine branch from 5807abb to e5e9b29 Compare February 26, 2024 21:10
@danieleades

Copy link
Copy Markdown
Contributor Author

@srittau I could really use your eyes on this one! I think i've made good progress, but could use some assistance to get this over the line, if you have the time

@github-actions

This comment has been minimized.

@danieleades

This comment was marked as resolved.

@danieleades
danieleades marked this pull request as ready for review February 27, 2024 07:24
@github-actions

This comment has been minimized.

2 similar comments
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@JelleZijlstra

Copy link
Copy Markdown
Member

i tried to simplify some of these type signatures using type aliases, but couldn't get this to work

_Context = TypeVar("_Context")
_TransitionResult: TypeAlias = tuple[_Context, str | None, list[str]]
_TransitionMethod: TypeAlias = Callable[[Match[str], _Context, str], _TransitionResult[_Context]]

mypy and pyright complain about the generics being unbound

That looks right, but you should use the alias as _TransitionResult[_Context] in this case.

@danieleades
danieleades force-pushed the docutils.statemachine branch from 32a7df9 to 6d80003 Compare March 2, 2024 08:02
@danieleades

This comment was marked as resolved.

@github-actions

This comment has been minimized.

@JelleZijlstra

Copy link
Copy Markdown
Member
stubs/docutils/docutils/statemachine.pyi:3: error: Module "typing" has no attribute "TypeAlias"  [attr-defined]
stubs/docutils/docutils/statemachine.pyi:3: note: Use `from typing_extensions import TypeAlias` instead

This probably has something to do with the other errors.

@danieleades
danieleades force-pushed the docutils.statemachine branch from f8eea52 to 1a308ad Compare March 2, 2024 15:02
@github-actions

This comment has been minimized.

@danieleades

Copy link
Copy Markdown
Contributor Author

@JelleZijlstra anything else needed to get this one merged? (it's blocking a couple of other PRs)

@danieleades
danieleades force-pushed the docutils.statemachine branch from 827d321 to b8897e9 Compare March 13, 2024 14:22
daniel.eades added 2 commits March 13, 2024 14:26
@github-actions

Copy link
Copy Markdown
Contributor

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

sphinx (https://github.com/sphinx-doc/sphinx)
+ sphinx/util/docutils.py: note: In function "switch_source_input":
+ sphinx/util/docutils.py:386:25: error: Need type annotation for "state_machine"  [var-annotated]
+ sphinx/util/docutils.py:386:25: note: Error code "var-annotated" not covered by "type: ignore" comment
- sphinx/util/docutils.py: note: At top level:
- sphinx/util/docutils.py:383: error: Unused "type: ignore" comment  [unused-ignore]
- sphinx/util/docutils.py:386: error: Unused "type: ignore" comment  [unused-ignore]
- sphinx/util/docutils.py:388: error: Unused "type: ignore" comment  [unused-ignore]
- sphinx/util/docutils.py:393: error: Unused "type: ignore" comment  [unused-ignore]
+ sphinx/util/rst.py: note: In function "append_epilog":
+ sphinx/util/rst.py:109:36: error: Unsupported operand types for + ("None" and "int")  [operator]
+ sphinx/util/rst.py:109:36: note: Left operand is of type "Optional[int]"
- sphinx/parsers.py:64: error: Unused "type: ignore" comment  [unused-ignore]

@srittau srittau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I spotted one thing. But looks good to go apart from that.

Comment thread stubs/docutils/docutils/statemachine.pyi Outdated
@github-actions

Copy link
Copy Markdown
Contributor

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

sphinx (https://github.com/sphinx-doc/sphinx)
+ sphinx/util/docutils.py: note: In function "switch_source_input":
+ sphinx/util/docutils.py:386:25: error: Need type annotation for "state_machine"  [var-annotated]
+ sphinx/util/docutils.py:386:25: note: Error code "var-annotated" not covered by "type: ignore" comment
- sphinx/util/docutils.py: note: At top level:
- sphinx/util/docutils.py:383: error: Unused "type: ignore" comment  [unused-ignore]
- sphinx/util/docutils.py:386: error: Unused "type: ignore" comment  [unused-ignore]
- sphinx/util/docutils.py:388: error: Unused "type: ignore" comment  [unused-ignore]
- sphinx/util/docutils.py:393: error: Unused "type: ignore" comment  [unused-ignore]
+ sphinx/util/rst.py: note: In function "append_epilog":
+ sphinx/util/rst.py:109:36: error: Unsupported operand types for + ("None" and "int")  [operator]
+ sphinx/util/rst.py:109:36: note: Left operand is of type "Optional[int]"
- sphinx/parsers.py:64: error: Unused "type: ignore" comment  [unused-ignore]

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.

5 participants