-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Adding stubs for ruamel.yaml
#12584
base: main
Are you sure you want to change the base?
Adding stubs for ruamel.yaml
#12584
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ruamel.yaml
ruamel.yaml
Currently stubtest fails to run: error: not checking stubs due to mypy build errors: /tmp/tmps3q1g0pn/lib/python3.11/site-packages/ruamel/yaml/main.py:47: error: Cannot assign to a type [misc] /tmp/tmps3q1g0pn/lib/python3.11/site-packages/ruamel/yaml/main.py:47: error: Incompatible types in assignment (expression has type "None", variable has type "type[CParser]") [assignment] /tmp/tmps3q1g0pn/lib/python3.11/site-packages/ruamel/yaml/main.py:47: error: Incompatible types in assignment (expression has type "None", variable has type "type[CEmitter]") [assignment] /tmp/tmps3q1g0pn/lib/python3.11/site-packages/ruamel/yaml/main.py:223: error: Unexpected keyword argument "loader" [call-arg] stubs/ruamel.yaml/_ruamel_yaml.pyi:22: note: Called function defined here /tmp/tmps3q1g0pn/lib/python3.11/site-packages/ruamel/yaml/main.py:223: error: Unexpected keyword argument "loader" [call-arg] stubs/ruamel.yaml/_ruamel_yaml.pyi:22: note: Called function defined here For the latter one, Not sure how to fix those. Stubsabot dry run also fails to run:
Help needed! |
This comment has been minimized.
This comment has been minimized.
Remarks on PyYAML is using the typeshed/stubs/PyYAML/yaml/nodes.pyi Lines 5 to 13 in dbe4d32
Sadly, the same trick can't be fully effective here. As you see, typeshed/stubs/ruamel.yaml/ruamel/yaml/error.pyi Lines 22 to 37 in 1f10574
|
This comment has been minimized.
This comment has been minimized.
Diff from mypy_primer, showing the effect of this PR on open source code: mitmproxy (https://github.com/mitmproxy/mitmproxy)
+ mitmproxy/optmanager.py:482: error: Need type annotation for "s" [var-annotated]
+ mitmproxy/tools/console/keymap.py:239: error: Item "StreamMark" of "Mark | StreamMark | None" has no attribute "get_snippet" [union-attr]
+ mitmproxy/tools/console/keymap.py:239: error: Item "None" of "Mark | StreamMark | None" has no attribute "get_snippet" [union-attr]
+ mitmproxy/tools/console/keymap.py:242: error: Item "None" of "Mark | StreamMark | None" has no attribute "line" [union-attr]
paasta (https://github.com/yelp/paasta)
+ paasta_tools/cli/cmds/validate.py:339: error: "Type[BaseConstructor]" has no attribute "flatten_mapping" [attr-defined]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! While I didn't check the types in depth, this looks very good to me. A few remarks below.
Also: We now require a comment explaining the use of Any
s (or an appropriate type alias). This can either list the allowed types, how the value is used, or any other reason why Any
is required.
version_info: tuple[int, int, int] | ||
__version__: str | ||
__with_libyaml__: bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The should most likely be final:
version_info: tuple[int, int, int] | |
__version__: str | |
__with_libyaml__: bool | |
version_info: Final[tuple[int, int, int]] | |
__version__: Final[str] | |
__with_libyaml__: Final[bool] |
(Needs import from typing
.)
# One of codecs.{utf_16_le_decode, utf_16_be_decode, utf_8_decode} | ||
class _BufferDecoder(Protocol): | ||
def __call__(data: ReadableBuffer, errors: str | None = None, final: bool = False, /) -> tuple[str, int]: ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# One of codecs.{utf_16_le_decode, utf_16_be_decode, utf_8_decode} | |
class _BufferDecoder(Protocol): | |
def __call__(data: ReadableBuffer, errors: str | None = None, final: bool = False, /) -> tuple[str, int]: ... | |
# One of codecs.{utf_16_le_decode, utf_16_be_decode, utf_8_decode} | |
@type_check_only | |
class _BufferDecoder(Protocol): | |
def __call__(data: ReadableBuffer, errors: str | None = None, final: bool = False, /) -> tuple[str, int]: ... |
(Needs import from typing.)
class _RepresenterFunction(Protocol[_Representer, _T_contra]): | ||
def __call__(self, dumper: _Representer, data: _T_contra, /) -> Node: ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class _RepresenterFunction(Protocol[_Representer, _T_contra]): | |
def __call__(self, dumper: _Representer, data: _T_contra, /) -> Node: ... | |
@type_check_only | |
class _RepresenterFunction(Protocol[_Representer, _T_contra]): | |
def __call__(self, dumper: _Representer, data: _T_contra, /) -> Node: ... |
ruamel.yaml
is a fork of PyYAML which features a new API and can perform round-trip conversion preserving the original format and comments. It also claims to have fixed various issues when the development of PyYAML was inactive.Typeshed testing depends on this package via pre-commit-hooks.
Although the package ships a
py.typed
file, almost every type hint isAny
, which makes coding experience terrible. This falls into the situation mentioned in #11955, which might need some discussion.The typing information in the package seems to originate from https://sourceforge.net/p/ruamel-yaml/tickets/42/ (from https://github.com/common-workflow-language/schema_salad) in 2016 when things were still in Python 2. Then it was never properly maintained and became a pile of
Any
s.This package doesn't follow the best practice of prefixing private properties and methods with underscore(s), which makes writing stubs a lot more complex due to the large number of "public" properties and methods.
It's also worth noting that the documentation isn't well-structured, nor does it cover necessary information like every format option, which makes it even harder to use this library. Adding proper typing information might provide some help.
Since there are already stubs for PyYAML, many modules ofThey turned out to be not very helpful.ruamel.yaml
can be added by referring to the corresponding ones in PyYAML.A special case in this library is the
YAML
class, which you need to pass atyp
argument to select its parsing mode. Depending on thetyp
, the methods ofYAML
behave differently. For example,YAML(typ='rt').seq()
returns aCommentedSeq
with round-trip-specific methods, whileYAML(typ='safe')
returns a plainlist
. To make coding easier, I added pseudo-subclasses ofYAML
(like_RoundTripYAML
) and converted the__init__()
method into multiple@overload def __new__()
methods which returns different variants based on thetyp
parameter.There's also an undocumented
rtsc
(round-trip split comments) type in the library, but the parsers seem to be very experimental and less-maintained, so I intentionally omitted it in themain
module. I found the only usage at https://github.com/SoulMelody/LibreSVIP/blob/main/libresvip/utils/yamlutils/__init__.py#L15 which doesn't seem to be intentional or necessary. Edit: The parser doesn't seem to be working at all on a YAML document with comments.Just for reference, I found the one and only
ruamel.yaml
plug-in code example at https://github.com/dstl/Stone-Soup/blob/main/stonesoup/serialise.py which might help understanding how thatplug_in
parameter works.MonkeyType provides great help.