Skip to content
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

Add stub file for xml/dom/xmlbuilder.py #6171

Merged
merged 12 commits into from
Nov 29, 2021

Conversation

AlexWaygood
Copy link
Member

I made a stab at adding stubs for xml/dom/xmlbuilder.py (cpython source code here). A little tricky, since documentation is rather thin on the ground, but perhaps an improvement on what's there at the moment?

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@AlexWaygood
Copy link
Member Author

The errors in the failing tests are:

  • "error: xml.dom.xmlbuilder.ExpatBuilder is not present at runtime"
  • "error: xml.dom.xmlbuilder.ExpatBuilderNS is not present at runtime"

Have I done something wrong there? The CONTRIBUTING.md file said: "Imports in stubs are considered private (not part of the exported API)."

Also, I included several methods that always raised errors, annotating them as returning NoReturn -- is that preferred, or should I have left them out?

@github-actions

This comment has been minimized.

@Akuli
Copy link
Collaborator

Akuli commented Oct 14, 2021

The CI failure looks like a bug to me. It is most likely a bug in mypy, as stubtest uses mypy internally. It might be python/mypy#10661

@hauntsaninja
Copy link
Collaborator

Okay, confirmed mypy bug. Here's the fix: python/mypy#11411

For now, I recommend adding it to stubtest's allowlist here: https://github.com/python/typeshed/blob/master/tests/stubtest_allowlists/py3_common.txt

If you then also want to run isort against this PR, CI will be green

@github-actions

This comment has been minimized.

@AlexWaygood
Copy link
Member Author

Okay, confirmed mypy bug. Here's the fix: python/mypy#11411

For now, I recommend adding it to stubtest's allowlist here: https://github.com/python/typeshed/blob/master/tests/stubtest_allowlists/py3_common.txt

If you then also want to run isort against this PR, CI will be green

🥳 CI is now green -- thanks!

Copy link
Collaborator

@Akuli Akuli left a comment

Choose a reason for hiding this comment

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

It seems that everyone somehow forgot about this :)

stdlib/xml/dom/xmlbuilder.pyi Outdated Show resolved Hide resolved
stdlib/xml/dom/xmlbuilder.pyi Outdated Show resolved Hide resolved
tests/stubtest_allowlists/py3_common.txt Outdated Show resolved Hide resolved
@AlexWaygood
Copy link
Member Author

It seems that everyone somehow forgot about this :)

The PR rises from the dead!

@Akuli
Copy link
Collaborator

Akuli commented Nov 28, 2021

You need to merge the latest master into the branch of this PR.

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Nov 28, 2021

You need to merge the latest master into the branch of this PR.

Oh, I think I understand how this happened. If it isn't the consequences of my own actions.

@Akuli
Copy link
Collaborator

Akuli commented Nov 28, 2021

This PR conflicted with #6379, which is not surprising because #6379 changed many lines.

Copy link
Collaborator

@Akuli Akuli left a comment

Choose a reason for hiding this comment

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

Much better than what was there before.

@AlexWaygood
Copy link
Member Author

Much better than what was there before.

Thank you for the patient reviews :)

@github-actions

This comment has been minimized.

5 similar comments
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@Akuli Akuli merged commit d5f9c95 into python:master Nov 29, 2021
@AlexWaygood
Copy link
Member Author

🙌🙌

@AlexWaygood AlexWaygood deleted the xmlbuilder-add-missing-stubs branch November 29, 2021 13:47
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