Change Loader to Type[BaseConstructor] in yaml.add_constructor#5828
Change Loader to Type[BaseConstructor] in yaml.add_constructor#5828Akuli merged 14 commits intopython:masterfrom
Conversation
|
Thank you! Feel free to change the other instances of this issue too. |
In the implementation of `yaml.add_constructor` the only requirement on the `Loader` parameter is that it defines the `Loader.add_constructor` class method. This method is defined in the `BaseConstructor` base class. So we should use this base class as type parameter type.
Added / modified types for the following functions: - `add_implicit_resolver(...)` - `add_path_resolver(...)` - `add_constructor(...)` - `add_multi_constructor(...)` - `add_representer(...)` - `add_multi_representer(...)` For the last two only some formatting was changed.
|
Hi @JelleZijlstra, thanks for having a look. I just added annotations to the other functions mentioned in the description. It would be good if someone more experience than me in both typing and the yaml-library could glance over it, just to see if all makes sense. Also, I'm not 100% sure if writing something like |
We need to use TypeVar in order to match the types in the second parameter (construtor) and third parameter (Loader). Setting `constructor: Callable[[BaseConstructor, None], Any]` and `Loader: OptionalType[BaseConstructor]` doesn't work for when `Loader` is a subclass `C` of `BaseConstructor`. This is because `Callable[[C, None], Any]` cannot be substituted for `Callable[[BaseConstructur], Any]`.
|
We changed all stubs to use the new Please also use the newer
It makes sense. PyYAML checks whether it is |
Hi @Akuli, thanks for reviewing. Merged master in bc77aed and also addressed your comment in the thread above. |
Version 5.4.6 contains a fix for wrong type annotations. The fix and additional type annotations were added in this PR: python/typeshed#5828
Version 5.4.6 contains a fix for wrong type annotations. The fix and additional type annotations were added in this PR: python/typeshed#5828
Version 5.4.6 contains a fix for wrong type annotations. The fix and additional type annotations were added in this PR: python/typeshed#5828
Version 5.4.6 contains a fix for wrong type annotations. The fix and additional type annotations were added in this PR: python/typeshed#5828
This fixes a wrong type in the PyYAML stubs.
The last parameter in
yaml.add_constructur(tag, constructor, Loader)is the loader class and not its instance. This is consistent with both the PyYAML docs and the way it's implemented in the PyYAML package.In fact, since the only requirement on the
Loaderparameter is to have theLoader.add_constructorclass method, which is defined in theBaseConstructorbase class, we should use the corresponding base class typeType[BaseConstructor]Simlar types can be added to the similar functions in the same block:
def add_implicit_resolver(...)def add_path_resolver(...)def add_constructor(...)def add_multi_constructor(...)def add_representer(...)def add_multi_representer(...)