Skip to content

[docutils] Improve types in docutils.io - #16260

Open
ekanshul wants to merge 3 commits into
python:mainfrom
ekanshul:docutils-io-types
Open

[docutils] Improve types in docutils.io#16260
ekanshul wants to merge 3 commits into
python:mainfrom
ekanshul:docutils-io-types

Conversation

@ekanshul

Copy link
Copy Markdown
Contributor

Several attributes and parameters of the docutils.io input/output classes were Incomplete or unannotated although docutils annotates them inline. This types the encoding, error_handler, destination_path and autoclose attributes of Output/FileOutput, the ErrorOutput attributes, the path/encoding/mode parameters of the FileInput and FileOutput constructors, and the read()/write() methods of StringInput, StringOutput and FileOutput, following the annotations in docutils 0.23.

Agent used: Claude Code

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@JelleZijlstra JelleZijlstra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are the mypy-primer hits in Sphinx expected?

Comment thread stubs/docutils/docutils/io.pyi Outdated
encoding: Incomplete
encoding_errors: Incomplete
decoding_errors: Incomplete
destination: TextIO | BinaryIO | Literal[False]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this be SupportsWrite[str] | SupportsWrite[bytes] | Literal[False] to mirror the constructor parameter?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 4cec7a9, and dropped the now unused BinaryIO import.

Whatever is passed to __init__ is stored, so mirror the parameter with
SupportsWrite[str] | SupportsWrite[bytes] | Literal[False] rather than
the concrete TextIO | BinaryIO.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@ekanshul

ekanshul commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Yes, they're expected. docutils 0.23 annotates FileOutput.write(self, data: str | bytes) -> str | bytes (and Output.write/StringOutput.write the same way), which is what this PR copies. SphinxFileOutput.write(self, data: str) -> str in sphinx/util/docutils.py narrows the parameter, so the override error is a true positive by docutils' own annotations, and the unused type: ignore[no-untyped-call] is the super().write(data) call becoming typed. Both look like things to adjust on the Sphinx side (widen the override, drop the ignore) rather than something to avoid here.

@github-actions

github-actions Bot commented Sep 9, 2026

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 member "write" of class "SphinxFileOutput":
+ sphinx/util/docutils.py:457:21: error: Argument 1 of "write" is incompatible with supertype "docutils.io.FileOutput"; supertype defines the argument type as "str | bytes"  [override]
+ sphinx/util/docutils.py:457:21: note: This violates the Liskov substitution principle
+ sphinx/util/docutils.py:457:21: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
+ sphinx/util/docutils.py: note: At top level:
+ sphinx/util/docutils.py:473: error: Unused "type: ignore" comment  [unused-ignore]
+ sphinx/util/docutils.py: note: In member "write" of class "SphinxFileOutput":
+ sphinx/util/docutils.py:473:16: error: Incompatible return value type (got "str | bytes", expected "str")  [return-value]
+ sphinx/util/docutils.py:473:16: note: Error code "return-value" not covered by "type: ignore[no-untyped-call]" comment

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