Unpin Sphinx by saimn · Pull Request #14093 · astropy/astropy

saimn · 2022-12-02T18:30:08Z

Ref #11725 and astropy/sphinx-automodapi#150

So basically using :noindex: with astropy/sphinx-automodapi#150 works in the sense that it removes the duplication warnings from Sphinx. But then for Astropy we get other warnings/errors about missing references ...

github-actions · 2022-12-02T18:30:47Z

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

saimn · 2022-12-02T18:55:48Z

Example of the new warnings with this PR:

.../astropy/coordinates/builtin_frames/hadec.py:docstring of astropy.coordinates.builtin_frames.hadec.HADec:1: WARNING: py:class reference target not found: astropy.coordinates.baseframe.BaseCoordinateFrame
.../astropy/coordinates/builtin_frames/hcrs.py:docstring of astropy.coordinates.builtin_frames.hcrs.HCRS:1: WARNING: py:class reference target not found: astropy.coordinates.builtin_frames.baseradec.BaseRADecFrame
.../astropy/coordinates/builtin_frames/ecliptic.py:docstring of astropy.coordinates.builtin_frames.ecliptic.HeliocentricEclipticIAU76:1: WARNING: py:class reference target not found: astropy.coordinates.builtin_frames.ecliptic.BaseEclipticFrame
.../astropy/coordinates/builtin_frames/ecliptic.py:docstring of astropy.coordinates.builtin_frames.ecliptic.HeliocentricMeanEcliptic:1: WARNING: py:class reference target not found: astropy.coordinates.builtin_frames.ecliptic.BaseEclipticFrame
.../astropy/coordinates/builtin_frames/ecliptic.py:docstring of astropy.coordinates.builtin_frames.ecliptic.HeliocentricTrueEcliptic:1: WARNING: py:class reference target not found: astropy.coordinates.builtin_frames.ecliptic.BaseEclipticFrame

pllim · 2022-12-02T22:19:05Z

@adrn , can we simplify coordinates? 😆

adrn · 2022-12-05T13:48:24Z

Hm, for the warnings in coordinates, is this because the Base*Frame API pages were being generated by the index made from automodapi'ing astropy.coordinates.builtin_frames?

pllim · 2022-12-05T17:43:30Z

@saimn or @larrybradley might remember...

saimn · 2022-12-06T18:13:08Z

See #14115.

mhvk · 2022-12-06T20:31:22Z

@saimn - can you re-open this? I fixed astropy.units at least which should give an idea about how to proceed... I tried pushing to your branch and suspect that failed because the PR is closed.

pllim · 2022-12-06T20:33:30Z

I reopened it though this might need a rebase by now...

saimn · 2022-12-06T21:02:13Z

I thought this PR was not needed/useful, do we still need :noindex: if we change the way to document those subpackages ?

mhvk · 2022-12-06T21:38:16Z

I'm using :noindex: in astropy.units.function. Perhaps we can indeed just not show it, but maybe it is good to just test that it works?

mhvk · 2022-12-06T22:23:13Z

I think I may have it down to one that I don't quite understand, but we'll see. Also, need to check whether :noindex: is indeed necessary, but time to start preparing some dinner now!

mhvk · 2022-12-07T00:45:28Z

OK, I was wrong -- still lots in coordinates. I think it does need a :noindex: in builtin_frames after all (somehow the local built seemed better). But at least some progress!

Cadair · 2022-12-07T11:23:04Z

Ok, so at least one of the issues with the coordinates stuff is the inheritance diagram, i.e. it generates a load of errors, I shall hunt down all the others and figure it out.

Cadair · 2022-12-07T14:19:59Z

omg it passed.

Cadair · 2022-12-07T14:45:56Z

 =============

 .. automodapi:: astropy.coordinates
+    :skip: ICRS


This obviously isn't great (it's the whole contents of the __all__). I don't think it substantially detracts from the documentation of this page (although maybe it needs a note to say things aren't listed here?). Ideally of course we would have a way of excluding objects on namespace, i.e. do something like :exclude-namespace: astropy.coordinates.builtin_frames but 🤷‍♂️

As long as we add a line that says "go here for frame classes" or something, I think this is acceptable if it is the only way we can unpin Sphinx max version.

The classes are listed in the block above, so it's not that hard to spot but I am happy to add a line.

Maybe good to see if we can change the transformgraph view to not link to classes - that seems to be the problem (and even now we cannot click on it anyway)

Is there actually a problem, other than the links inside the graph not working? (I am not sure they did to begin with)

I thought so for a bit, but now am really unsure what is actually causing what problems. Really, these many skip shouldn't be necessary...

pllim · 2022-12-07T15:10:15Z

@@ -509,11 +510,12 @@ def __init__(self, *args, **kwargs):

    @classmethod
    def get_roll0(cls):


Unrelated... but we might need a get_roll20 🤓

pllim · 2022-12-07T15:16:27Z

            "astropy": None,
            "matplotlib": "https://matplotlib.org/stable/",
-            "numpy": "https://numpy.org/doc/stable/",
+            # The numpy search js isn't loadable at the moment (2022-12-07)


Wait, what?

This was causing an error, and it's because the JS is missing some quotes apparently 🤷‍♂️

But I don't see any such failure in other PRs. Is the JS only used for this PR?

Oooo is the JS only used if you unpin Sphinx? Is this something we have to report to numpy?

pllim · 2022-12-07T15:18:38Z

    s3fs>=2022.8.2
 docs =
-    sphinx<4
+    sphinx


Also change the Jinja2 pin back to this:

Jinja2>=3.0

mhvk · 2022-12-07T15:22:22Z

Nice!!! I think now you narrowed it down we should be able to get rid of the problems with the graph. It seems like we do not really need :noindex: -- I think we can simply omit astropy.units.function (though will think a bit more about that).

pllim · 2022-12-07T15:24:15Z

Given all the work Simon has poured into making noindex work, I feel like we are obligated to use it. 😆

larrybradley

Note that Sphinx 5.2 introduced a new feature (that is on by default) with a table of contents for functions and classes. That text overflows the left side bar window in the astropy sphinx theme. See, e.g., from this PR: https://astropy--14093.org.readthedocs.build/en/14093/api/astropy.table.Table.html#astropy.table.Table

Fortunately, Sphinx 5.2.3 added a configuration option to turn it off: set toc_object_entries = False in docs.conf.

xref: astropy/photutils#1424
xref: https://github.com/astropy/photutils/pull/1465/files

mhvk · 2022-12-07T18:13:58Z

I think I've got an alternative without :noindex: by using automodule instead of automodapi. Maybe easiest to make a separate PR so it is easier to compare.

as suggested by larrybradley

pllim · 2022-12-07T18:23:51Z

I pushed a commit to address #14093 (review) . Thanks, @larrybradley !

mhvk · 2022-12-07T19:06:55Z

See #14140 for my alternative -- I think it is cleaner.

Cadair · 2022-12-09T10:18:28Z

Closed in favour of #14140

saimn added the Docs label Dec 2, 2022

github-actions Bot added coordinates installation units labels Dec 2, 2022

saimn mentioned this pull request Dec 6, 2022

Compatibility with Sphinx ≥ 4 #14115

Closed

saimn closed this Dec 6, 2022

pllim reopened this Dec 6, 2022

saimn and others added 5 commits December 6, 2022 17:22

Unpin Sphinx

06d3277

Install dev package [ci skip]

74709f6

Get rid of duplicate/missing reference warnings for astropy.units

153f999

Ensure references in WCS utils are canonical

b16916c

Fix up coordinates references

f74a974

mhvk force-pushed the sphinx4 branch from 18b7d19 to f74a974 Compare December 6, 2022 22:22

Try something

7585691

Cadair force-pushed the sphinx4 branch from e41dc48 to 7585691 Compare December 7, 2022 10:30

Ok, this might have worked on my local machine

e75b643

Cadair reviewed Dec 7, 2022

View reviewed changes

pllim reviewed Dec 7, 2022

View reviewed changes

Comment thread setup.cfg

s3fs>=2022.8.2

docs =

sphinx<4

sphinx

pllim Dec 7, 2022

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

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

Also change the Jinja2 pin back to this:

Jinja2>=3.0

larrybradley reviewed Dec 7, 2022

View reviewed changes

DOC: Fix TOC overrun

c175414

as suggested by larrybradley

mhvk mentioned this pull request Dec 7, 2022

Unpin sphinx alternative #14140

Merged

8 of 10 tasks

Cadair closed this Dec 9, 2022

saimn deleted the sphinx4 branch December 11, 2022 17:59

		@@ -509,11 +510,12 @@ def __init__(self, args, *kwargs):

		@classmethod
		def get_roll0(cls):

Conversation

saimn commented Dec 2, 2022

Uh oh!

github-actions Bot commented Dec 2, 2022

Uh oh!

saimn commented Dec 2, 2022

Uh oh!

pllim commented Dec 2, 2022

Uh oh!

adrn commented Dec 5, 2022

Uh oh!

pllim commented Dec 5, 2022

Uh oh!

saimn commented Dec 6, 2022

Uh oh!

mhvk commented Dec 6, 2022

Uh oh!

pllim commented Dec 6, 2022

Uh oh!

saimn commented Dec 6, 2022

Uh oh!

mhvk commented Dec 6, 2022

Uh oh!

mhvk commented Dec 6, 2022

Uh oh!

mhvk commented Dec 7, 2022

Uh oh!

Cadair commented Dec 7, 2022

Uh oh!

Cadair commented Dec 7, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhvk commented Dec 7, 2022

Uh oh!

pllim commented Dec 7, 2022

Uh oh!

larrybradley left a comment

Choose a reason for hiding this comment

Uh oh!

mhvk commented Dec 7, 2022

Uh oh!

pllim commented Dec 7, 2022

Uh oh!

mhvk commented Dec 7, 2022

Uh oh!

Cadair commented Dec 9, 2022

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants