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

Shrink mypy whitelist for 'util.nodes' module #11061

Merged
merged 1 commit into from Jan 2, 2023

Conversation

danieleades
Copy link
Contributor

No description provided.

@AA-Turner
Copy link
Member

This is a breaking change; consumers of the API do not expect errors here.

More generally, Sphinx has not been very good at delineating public vs private API in the past and as such extensions, themes, etc often use APIs that are not documented as public. I would like to clean this up, but it takes time and deprecations.

A

@danieleades
Copy link
Contributor Author

danieleades commented Dec 31, 2022

This is a breaking change; consumers of the API do not expect errors here.

More generally, Sphinx has not been very good at delineating public vs private API in the past and as such extensions, themes, etc often use APIs that are not documented as public. I would like to clean this up, but it takes time and deprecations.

A

Thanks for the input. That's what I've been trying to figure out- since the None case is already not handled, I'm thinking this would already be an exception if it was encountered. I was guessing that maybe returning None was just a hack to keep mypy happy (prior to the introduction of "strict optional"), but not actually a viable path

As for the demarcation of public and private objects- it would be good to introduce "__all__" to each module to make that clear. In my experience, linter support for "__all__" is patchy, but improving.

@AA-Turner
Copy link
Member

Having looked at GitHub's code search, it appears these utility function are not used outwith Sphinx itself.

A

@AA-Turner AA-Turner changed the title shrink mypy whitelist for util.nodes module Shrink mypy whitelist for 'util.nodes' module Jan 2, 2023
@AA-Turner AA-Turner merged commit 256e521 into sphinx-doc:master Jan 2, 2023
@danieleades
Copy link
Contributor Author

More generally, Sphinx has not been very good at delineating public vs private API in the past and as such extensions, themes, etc often use APIs that are not documented as public. I would like to clean this up, but it takes time and deprecations.

one way to start making inroads on this is to add to the mypy config-

no_implicit_reexport = true

That should help straighten out some of the spaghetti

@danieleades danieleades deleted the typing-util/nodes branch January 3, 2023 08:19
@gaborbernat
Copy link
Contributor

This seem to introduced a bug here #11091

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants