-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Manual changes of Any
union to Incomplete
in stubs folder
#9566
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
Conversation
- ClassVar[Any | None] - Missed previous changes due to alias - Manual review of leftover Any unions (`| Any` and `Any |`)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Any
to Incomplete in stubs folderAny
union to Incomplete
in stubs folder
stubs/toml/toml/encoder.pyi
Outdated
def dump_inline_table(self, section: dict[str, Any] | Any) -> str: ... | ||
def dump_value(self, v: Any) -> str: ... | ||
def dump_inline_table(self, section: object) -> str: ... | ||
def dump_value(self, v: object) -> str: ... |
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.
Are you sure this should be object
? I don't think toml can dump all objects.
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.
From what I can see of the implementation, and from trying it, if it doesn't specifically handle an object it just stringifies it.
>>> TomlEncoder().dump_inline_table(object())
'"<object object at 0x000001FF99FF85A0>"'
>>> TomlEncoder().dump_value(object())
'"<object object at 0x000001FF99FF85A0>"'
>>> TomlEncoder().dump_value(dict())
'[]'
>>> TomlEncoder().dump_value([object()])
'[ "<object object at 0x000001FF99FF85A0>",]'
>>> TomlEncoder().dump_inline_table([object()])
'[ "<object object at 0x000001FF99FF85B0>",]'
>>> TomlEncoder().dump_inline_table({"aaa": object()})
'{ aaa = "<object object at 0x000001FF99FF85A0>" }\n'
>>> TomlEncoder().dump_value({"aaa": object()})
'[ "aaa",]'
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.
While technically it can handle all objects, in that it doesn't throw an exception, not all objects are correct arguments. "<object object at 0x000001FF99FF85B0>"
is obviously not useful as an output.
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.
And trying to represent intended values, we kindof hit the same issue as JSON.
There's also classes with custom repr that can be dumped.
And subclassed Encoder that extend dump_funcs
to support more intended types.
To be consistent though, how about the dump_list
method typed with Iterable[object]
? I feel the same logic should apply and be Iterable[Any]
. (it's calling dump_value
internally)
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.
That's why Any makes sense here: the type annotation we'd ideally want isn't representable in the type system.
stubs/PyYAML/yaml/nodes.pyi
Outdated
from typing import Any, ClassVar | ||
|
||
from yaml.error import Mark | ||
|
||
class Node: | ||
tag: str | ||
value: Any | ||
start_mark: Mark | Any | ||
end_mark: Mark | Any | ||
start_mark: Mark | Incomplete |
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.
This kind of looks like it's using the | Any
trick for when the type is almost always one particular type (like we do for re.match
). If so, Any
is right.
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.
Those attributes are directly assigned from the parameters, so they should be the same.
If it's possible, and intended to be anything else than Mark | None
, with an arbitrary type, or if it's to avoid having to check for a None mark when you know you instantiated it not-None. And that's a common use-case/pattern. That can be represented using generics. Since it's feasible within the current typing system, Incomplete
(could be done but we didn't bothered) seems preferable over Any
(can't be done, we tried)
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.
I think there's an argument to be made here for either permissive unions or default generics. Which both aren't part of the type system atm. I've changed the type to Mark | None | Any
Mark | Any
and added a comment to explain.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Follow up to #9565 . Ref #9550
| Any
andAny |
)Avoid touching any return type (function or
Callable
). Or when a comment explained the reason behind usingAny
.After this PR, all
Any
unions left can basically stay untouched. So we can avoid changing any of 'em, or return types, in the next stage.