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 'paragraph' node to docutils #10102

Merged
merged 5 commits into from
May 3, 2023

Conversation

danieleades
Copy link
Contributor

@danieleades danieleades commented Apr 29, 2023

No description provided.

@danieleades danieleades force-pushed the docutils.nodes.paragraph branch from 22bb753 to b72a28a Compare April 29, 2023 09:07
@danieleades danieleades marked this pull request as draft April 29, 2023 09:08
@danieleades danieleades force-pushed the docutils.nodes.paragraph branch from b72a28a to e6cac64 Compare April 29, 2023 09:08
@github-actions

This comment has been minimized.

@danieleades
Copy link
Contributor Author

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

sphinx (https://github.com/sphinx-doc/sphinx)
+ sphinx/util/docfields.py: note: In function "make_field":
+ sphinx/util/docfields.py:216:20: error: Argument 1 to "__iadd__" of "Element" has incompatible type "str"; expected "Union[Node, Iterable[Node]]"  [arg-type]
+ sphinx/util/docfields.py:216:20: note: Following member(s) of "str" have conflicts:
+ sphinx/util/docfields.py:216:20: note:     Expected:
+ sphinx/util/docfields.py:216:20: note:         def __iter__(self) -> Iterator[Node]
+ sphinx/util/docfields.py:216:20: note:     Got:
+ sphinx/util/docfields.py:216:20: note:         @overload
+ sphinx/util/docfields.py:216:20: note:         def __iter__(self) -> Iterator[str]
+ sphinx/util/docfields.py:216:20: note:         @overload
+ sphinx/util/docfields.py:216:20: note:         def __iter__(self) -> Iterator[str]
+ sphinx/util/docfields.py: note: In member "transform_all" of class "DocFieldTransformer":
+ sphinx/util/docfields.py:246:22: error: "desc_content" has no attribute "__iter__" (not iterable)  [attr-defined]
+ sphinx/domains/index.py: note: In member "process_doc" of class "IndexDomain":
+ sphinx/domains/index.py:51:17: error: Item "Node" of "Optional[Node]" has no attribute "remove"  [union-attr]
+ sphinx/domains/index.py:51:17: error: Item "None" of "Optional[Node]" has no attribute "remove"  [union-attr]
+ sphinx/domains/changeset.py: note: In member "note_changeset" of class "ChangeSetDomain":
+ sphinx/domains/changeset.py:122:63: error: Argument 3 to "ChangeSet" has incompatible type "Optional[int]"; expected "int"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "apply" of class "NestedInlineTransform":
+ sphinx/writers/manpage.py:53:9: error: Incompatible types in assignment (expression has type "Node", variable has type "TextElement")  [assignment]
+ sphinx/writers/manpage.py:54:52: error: "TextElement" has no attribute "__iter__" (not iterable)  [attr-defined]
+ sphinx/writers/manpage.py:55:23: error: Item "Node" of "Optional[Node]" has no attribute "index"  [union-attr]
+ sphinx/writers/manpage.py:55:23: error: Item "None" of "Optional[Node]" has no attribute "index"  [union-attr]
+ sphinx/writers/manpage.py:59:25: error: Item "Node" of "Optional[Node]" has no attribute "insert"  [union-attr]
+ sphinx/writers/manpage.py:59:25: error: Item "None" of "Optional[Node]" has no attribute "insert"  [union-attr]
+ sphinx/writers/manpage.py:62:25: error: Item "Node" of "Optional[Node]" has no attribute "insert"  [union-attr]
+ sphinx/writers/manpage.py:62:25: error: Item "None" of "Optional[Node]" has no attribute "insert"  [union-attr]
+ sphinx/writers/manpage.py:65:21: error: Item "Node" of "Optional[Node]" has no attribute "remove"  [union-attr]
+ sphinx/writers/manpage.py:65:21: error: Item "None" of "Optional[Node]" has no attribute "remove"  [union-attr]
+ sphinx/writers/manpage.py: note: At top level:
+ sphinx/transforms/post_transforms/code.py: note: In member "strip_doctest_flags" of class "TrimDoctestFlagsTransform":
+ sphinx/transforms/post_transforms/code.py:105:9: error: "TextElement" has no attribute "rawsource"  [attr-defined]

No good deed goes unpunished

@danieleades danieleades force-pushed the docutils.nodes.paragraph branch from 6a6643c to abbcf2b Compare April 29, 2023 14:49
@github-actions

This comment has been minimized.

@danieleades danieleades marked this pull request as ready for review April 29, 2023 15:00
@github-actions

This comment has been minimized.

@danieleades danieleades requested a review from AlexWaygood April 29, 2023 15:18
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

The mypy_primer diff is big, but this looks correct, and I think the new errors are all unavoidable. I'd love a second opinion from another maintainer though.

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

This does look correct, but it would be good to go over the Sphinx hits in more detail to figure out what's going on with them.

@github-actions

This comment has been minimized.

@danieleades danieleades force-pushed the docutils.nodes.paragraph branch from 24bb0e1 to a492aaf Compare April 30, 2023 06:42
@github-actions

This comment has been minimized.

@danieleades
Copy link
Contributor Author

ping @AA-Turner, @jfbu, @tk0miya

@AlexWaygood
Copy link
Member

AlexWaygood commented Apr 30, 2023

ping @AA-Turner, @jfbu, @tk0miya

Hmm, it's not the Sphinx maintainers' responsibility to help us figure this out, especially since I don't think they even use our stubs for docutils in their CI :)

I'll try to take a deeper look when I have a chance, but might not have the time this weekend

@danieleades
Copy link
Contributor Author

ping @AA-Turner, @jfbu, @tk0miya

Hmm, it's not the Sphinx maintainers' responsibility to help us figure this out, especially since I don't think they even use our stubs for docutils in their CI :)

I'll try to take a deeper look when I have a chance, but might not have the time this weekend

They've been very invested in improving typing support in sphinx, so I think this will be of interest

@AA-Turner
Copy link
Member

Hmm, it's not the Sphinx maintainers' responsibility to help us figure this out, especially since I don't think they even use our stubs for docutils in their CI :)

Thanks Alex, Sphinx uses docutils-stubs. Docutils will probably get some type annotations in v0.21.

A

@github-actions
Copy link
Contributor

github-actions bot commented May 1, 2023

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

sphinx (https://github.com/sphinx-doc/sphinx)
- sphinx/util/nodes.py: note: In function "apply_source_workaround":
- sphinx/util/nodes.py:161:9: error: "Element" has no attribute "rawsource"  [attr-defined]
- sphinx/util/nodes.py: note: At top level:
+ sphinx/util/docfields.py: note: In function "make_field":
+ sphinx/util/docfields.py:216:20: error: Argument 1 to "__iadd__" of "Element" has incompatible type "str"; expected "Union[Node, Iterable[Node]]"  [arg-type]
+ sphinx/util/docfields.py:216:20: note: Following member(s) of "str" have conflicts:
+ sphinx/util/docfields.py:216:20: note:     Expected:
+ sphinx/util/docfields.py:216:20: note:         def __iter__(self) -> Iterator[Node]
+ sphinx/util/docfields.py:216:20: note:     Got:
+ sphinx/util/docfields.py:216:20: note:         @overload
+ sphinx/util/docfields.py:216:20: note:         def __iter__(self) -> Iterator[str]
+ sphinx/util/docfields.py:216:20: note:         @overload
+ sphinx/util/docfields.py:216:20: note:         def __iter__(self) -> Iterator[str]
+ sphinx/domains/index.py: note: In member "process_doc" of class "IndexDomain":
+ sphinx/domains/index.py:51:17: error: Item "Node" of "Optional[Node]" has no attribute "remove"  [union-attr]
+ sphinx/domains/index.py:51:17: error: Item "None" of "Optional[Node]" has no attribute "remove"  [union-attr]
+ sphinx/domains/changeset.py: note: In member "note_changeset" of class "ChangeSetDomain":
+ sphinx/domains/changeset.py:122:63: error: Argument 3 to "ChangeSet" has incompatible type "Optional[int]"; expected "int"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "apply" of class "NestedInlineTransform":
+ sphinx/writers/manpage.py:53:9: error: Incompatible types in assignment (expression has type "Node", variable has type "TextElement")  [assignment]
+ sphinx/writers/manpage.py:55:23: error: Item "Node" of "Optional[Node]" has no attribute "index"  [union-attr]
+ sphinx/writers/manpage.py:55:23: error: Item "None" of "Optional[Node]" has no attribute "index"  [union-attr]
+ sphinx/writers/manpage.py:59:25: error: Item "Node" of "Optional[Node]" has no attribute "insert"  [union-attr]
+ sphinx/writers/manpage.py:59:25: error: Item "None" of "Optional[Node]" has no attribute "insert"  [union-attr]
+ sphinx/writers/manpage.py:62:25: error: Item "Node" of "Optional[Node]" has no attribute "insert"  [union-attr]
+ sphinx/writers/manpage.py:62:25: error: Item "None" of "Optional[Node]" has no attribute "insert"  [union-attr]
+ sphinx/writers/manpage.py:65:21: error: Item "Node" of "Optional[Node]" has no attribute "remove"  [union-attr]
+ sphinx/writers/manpage.py:65:21: error: Item "None" of "Optional[Node]" has no attribute "remove"  [union-attr]
+ sphinx/writers/manpage.py: note: At top level:

@danieleades
Copy link
Contributor Author

looks like the majority of these errors are perfectly valid. Sphinx has only had mypy strict_optional for a short while, so there are still many of these errors. Probably a good chunk of these were masked by the lack of type stubs for these methods.

the iadd error is more interesting...

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

Considering this PR only adds previously missing items and those look correct to me, I'm willing to ignore the Spinx hits for now.

@srittau srittau merged commit 6b5ca0b into python:main May 3, 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.

5 participants