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

Force Sphinx 4 #38

Merged
merged 4 commits into from May 22, 2021
Merged

Force Sphinx 4 #38

merged 4 commits into from May 22, 2021

Conversation

alexfikl
Copy link
Collaborator

Was trying to see if the :canonical: thing in Sphinx 4 actually works. At least locally, this seems to generate the docs without warnings.

@alexfikl
Copy link
Collaborator Author

I was looking through the docs to see if this made anything look fishy and saw that setting autodoc_typehints = "description" makes the singledispatch-ed functions impressively useless.

Screenshot_20210521_191744

Should I disable it?

@alexfikl
Copy link
Collaborator Author

8ed6c8a is unrelated and can be spun off into a separate PR (?). I just added them while trying to convince sphinx to add annotations selectively (didn't work).

@inducer
Copy link
Owner

inducer commented May 22, 2021

I was looking through the docs to see if this made anything look fishy and saw that setting autodoc_typehints = "description" makes the singledispatch-ed functions impressively useless.

Yikes! Why would it remove the types from the signature? That's dumb. Let's get rid of it. (21eab4b)

Btw, I think we went through pretty much the same process with :canonical:. I was all excited about it and tried adding it, only to discover that (a) its effect is thoroughly underwhelming and (b) Sphinx 4 seems to do the right thing out of the box.

@inducer
Copy link
Owner

inducer commented May 22, 2021

Also, thanks for working on this!

@inducer
Copy link
Owner

inducer commented May 22, 2021

Also, why not: inducer/ci-support@e0402f6,
9581550.

@inducer
Copy link
Owner

inducer commented May 22, 2021

8ed6c8a is great btw. Thanks for that. It can totally stay.

@inducer inducer enabled auto-merge (rebase) May 22, 2021 22:59
@alexfikl
Copy link
Collaborator Author

Yikes! Why would it remove the types from the signature? That's dumb. Let's get rid of it. (21eab4b)

It sort of does what it says on the tin, i.e. "description" moves the annotations from the signature to the parameter list. Unfortunately that's a pretty bad idea for singledispatch-ed functions :(

Btw, I think we went through pretty much the same process with :canonical:. I was all excited about it and tried adding it, only to discover that (a) its effect is thoroughly underwhelming and (b) Sphinx 4 seems to do the right thing out of the box.

Yeah, it seems that it adds :canonical: itself for all the autosomething directives, which is pretty nifty! The only annoying thing is that it seems to keep the full names, e.g. modepy.shapes.Simplex instead of just modepy.Simplex in the type annotations :\

@inducer inducer merged commit 97cd59d into inducer:main May 22, 2021
@alexfikl alexfikl deleted the sphinx4-canonical branch May 22, 2021 23:16
inducer added a commit to inducer/pyvisfile that referenced this pull request May 22, 2021
inducer added a commit to inducer/pytato that referenced this pull request May 22, 2021
inducer added a commit to inducer/loopy that referenced this pull request May 22, 2021
inducer added a commit to inducer/leap that referenced this pull request May 22, 2021
inducer added a commit to inducer/dagrt that referenced this pull request May 22, 2021
inducer added a commit to inducer/cgen that referenced this pull request May 22, 2021
inducer added a commit to inducer/sumpy that referenced this pull request May 22, 2021
inducer added a commit to inducer/arraycontext that referenced this pull request May 22, 2021
inducer added a commit to inducer/meshmode that referenced this pull request May 22, 2021
inducer added a commit to inducer/islpy that referenced this pull request May 22, 2021
inducer added a commit to inducer/grudge that referenced this pull request May 22, 2021
inducer added a commit to inducer/gmsh_interop that referenced this pull request May 22, 2021
inducer added a commit to inducer/arraycontext that referenced this pull request May 22, 2021
inducer added a commit to inducer/pytato that referenced this pull request May 22, 2021
@inducer
Copy link
Owner

inducer commented May 22, 2021

Yeah, it seems that it adds :canonical: itself for all the autosomething directives, which is pretty nifty! The only annoying thing is that it seems to keep the full names, e.g. modepy.shapes.Simplex instead of just modepy.Simplex in the type annotations :\

IME it doesn't if you add .. currentmodule:: modepy above?

inducer added a commit to inducer/dagrt that referenced this pull request May 22, 2021
inducer added a commit to inducer/gmsh_interop that referenced this pull request May 22, 2021
inducer added a commit to inducer/arraycontext that referenced this pull request May 22, 2021
@alexfikl
Copy link
Collaborator Author

IME it doesn't if you add .. currentmodule:: modepy above?

I meant the way it shows up below, for example. That chunk of docs should have a currentmodule:: modepy above it and the docs for the Simplex class show up as modepy.Simplex, but the type annotations still have the full path.

Screenshot_20210522_183513

inducer added a commit to inducer/arraycontext that referenced this pull request May 22, 2021
@inducer
Copy link
Owner

inducer commented May 22, 2021

OIC. But if you click, you go to modepy.Simplex, which is already huge. I imagine they'll get the type annotations fixed up eventually.

inducer added a commit to inducer/loopy that referenced this pull request May 22, 2021
@alexfikl
Copy link
Collaborator Author

OIC. But if you click, you go to modepy.Simplex, which is already huge. I imagine they'll get the type annotations fixed up eventually.

Definitely, I'm very happy with the improvements already. Don't get to have a negative diff very often! 🎉

inducer added a commit to inducer/pytools that referenced this pull request May 22, 2021
inducer added a commit to inducer/sumpy that referenced this pull request May 22, 2021
inducer added a commit to inducer/pyopencl that referenced this pull request May 23, 2021
inducer added a commit to inducer/pymbolic that referenced this pull request May 23, 2021
inducer added a commit to inducer/grudge that referenced this pull request May 23, 2021
@alexfikl
Copy link
Collaborator Author

@inducer Potentially interesting until they update the type annotation
https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-python_use_unqualified_type_names

inducer added a commit to inducer/meshmode that referenced this pull request May 23, 2021
inducer added a commit to inducer/islpy that referenced this pull request May 23, 2021
inducer added a commit to inducer/pytential that referenced this pull request May 23, 2021
@inducer
Copy link
Owner

inducer commented May 23, 2021

@inducer Potentially interesting until they update the type annotation
https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-python_use_unqualified_type_names

Thanks for the suggestion! The current state is usable though IMO, no way I'm doing another 30 PRs. :)

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.

None yet

2 participants