Skip to content

Add FullLoader class to third_party/2and3/yaml/loader.pyi - #3035

Merged
srittau merged 2 commits into
python:masterfrom
markedwards:add_yaml_fullloader
Jun 6, 2019
Merged

srittau merged 2 commits into
python:masterfrom
markedwards:add_yaml_fullloader

Conversation

@markedwards

Copy link
Copy Markdown

The yaml module has added the FullLoader class, and it is recommended in their documentation when calling yaml.load(). Adding a type definition here.

@srittau srittau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you, one remark.

Comment thread third_party/2and3/yaml/loader.pyi Outdated
class BaseLoader(Reader, Scanner, Parser, Composer, BaseConstructor, BaseResolver):
def __init__(self, stream) -> None: ...

class FullLoader(Reader, Scanner, Parser, Composer, SafeConstructor, Resolver):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This actually derives from FullConstructor, which is a sub-class of SafeConstructor. Since we don't have stubs for that class, yet, I see two options:

  • Add FullConstructor to yaml.constructor.pyi, possible marked with # incomplete and use that.
  • Just add a comment explaining that FullConstructor would be correct here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Doh! Thanks for catching that. Fixed in a second commit.

@srittau srittau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@srittau
srittau merged commit 5c94b58 into python:master Jun 6, 2019
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.

2 participants