intersphinx xref #40

nstarman · 2021-05-01T23:13:33Z

Signed-off-by: Nathaniel Starkman (@nstarman) [email protected]

Add section on numpydoc xref to v1.
preload the xref ignore set. This can be modified or replaced in packages with standard set operations.
Add numpydoc_xref_aliases_astropy_glossary, an xref mapping that can be added to the numpydoc_xref_aliases in packages by dictionary updating. This is designed for easy inclusion of astropy's glossary.
Add numpydoc_xref_aliases_astropy_physical_type, an xref mapping that can be added to the numpydoc_xref_aliases in packages by dictionary updating. This is designed for easy inclusion of astropy's physical-type x-refs.
Add numpydoc_xref_astropy_aliases, an xref ChainMap of the above mappings that can be added to the numpydoc_xref_aliases in packages by dictionary updating. This is designed for easy inclusion of ALL of astropy's intersphinx offerings.

Fixes #39
Fixes #20
Request Review : @bsipocz @pllim @saimn @astrofrog

sphinx_astropy/conf/v1.py

setup.cfg

sphinx_astropy/conf/v1.py

nstarman · 2021-05-02T18:43:13Z

astropy/astropy#11684 shows that this will not break astropy due to circular imports.
I don't think this will be super testable until astropy 4.3 is the 'stable' version in the intersphinx connection. @adrn and I have a PR in gala to test this when v4.3 happens. astropy/astropy#11684 is likewise set up for testing this PR in astropy itself (with intersphinx fixes in #astropy/astropy#11690).

sphinx_astropy/conf/v1.py

saimn · 2021-05-04T20:32:15Z

I'm not convinced about the cross-reference/dependency with astropy, this may cause problems in the future. Could we put a static list instead ? I guess the list doesn't change often so we could avoid the dynamic generation.
Also numpydoc xref should be disabled by default (which is the case here I think), and we could just provide numpydoc_xref_aliases_astropy_glossary and numpydoc_xref_aliases_astropy_physical_type (no matter the astropy version) and tell people to include what they need in numpydoc_xref_aliases.

nstarman · 2021-05-04T21:01:39Z

I'm not convinced about the cross-reference/dependency with astropy, this may cause problems in the future.

Do you mean having astropy be a run-time requirement of this package? I was initially concerned (hence astropy/astropy#11684) as I wasn't sure that the local package (astropy) got installed before the documentation requirements, but since it does, there shouldn't be any issues unless the installation order changes. I think PIP and Conda handle that, or does TOX also have a say? I feel good about PIP and Conda, but am not overly familiar with tox.

Could we put a static list instead ? I guess the list doesn't change often so we could avoid the dynamic generation.

The issue then is that any time astropy wants to update the list, sphinx-astropy needs a new release. Dynamic generation obviates the maintainer burden.

Also numpydoc xref should be disabled by default (which is the case here I think)

Agreed. It is here. I have a comment explaining how to turn it on.

and we could just provide numpydoc_xref_aliases_astropy_glossary and numpydoc_xref_aliases_astropy_physical_type (no matter the astropy version) and tell people to include what they need in numpydoc_xref_aliases.

I don't think I understand what you're suggesting here. Could you please clarify what would go into numpydoc_xref_aliases? Right now I have it aggregate all the astropy additions to numpydoc xref. Thanks!

One large danger with adding sphinx links to the numpydoc xref regardless of astropy version is that they can cause the docs to fail, depending on the astropy version. I don't think NOT including them will fail a numpydoc xref unless the the docs are built with an earlier version of astropy than is supported in the code.

nstarman · 2021-05-25T00:30:04Z

ping @astrofrog @pllim @saimn @bsipocz.
I've cleaned up the dynamic generation and now that astropy/astropy#11690 is in, the intersphinx links in numpydoc_xref_aliases_astropy_glossary, and numpydoc_xref_aliases_astropy_physical_type can programagically work in Astropy! It should be convenient for both Astropy and affiliate packages. The Astropy side will be implemented with astropy/astropy#11684.

astrofrog

I am in agreement with @saimn that we shouldn't have astropy as a dependency here - instead we should just make use of it if it is installed but not otherwise. For most packages that use this astropy will be a required dependency anyway, and for astropy core we will avoid installing a stable astropy into a development build of the astropy docs.

To be clear, is there any risk of this breaking anything which would warrant a v2 config? I suspect not since this is opt-in, but just wanted to check.

sphinx_astropy/conf/v1.py

nstarman · 2021-06-01T16:02:31Z

@saimn, I've incorporated @astrofrog's suggestion to make Astropy an optional dependency. The xref dictionaries are empty if Astropy is not installed or <v4.3 and full otherwise. Does this address the points you raised about cross-dependencies and working around that by static lists?

saimn

Thanks @nstarman, yes I agree the approach seems reasonable now. Just needs a small change.

saimn · 2021-06-14T19:59:59Z

sphinx_astropy/conf/v1.py

+# convenience.
+numpydoc_xref_ignore = {
+    "type", "optional", "default", "or", "of", "method", "instance",
+    "class", "subclass", "keyword-only", "default", "thereof",


default is listed twice.

Great. I'll push the change.

saimn

Thanks @nstarman, sounds good now.

@astrofrog - Do you want to have another look ?

@nstarman

Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>

@nstarman

Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>

@nstarman

Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>

@nstarman

Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>

@nstarman

Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>

@nstarman

cleanup dynamic generation Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>

@nstarman

The version check only runs if astropy is installed. Also update numpydoc xref ignores, from review comment. Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>

nstarman · 2021-06-17T16:28:36Z

Given the approval, I've squashed some unnecessary commits.
Also, I changed the version check from float(astropy.__version__[:3]) >= 4.3 to minversion(astropy, "4.3"). Much cleaner.

GTG.

saimn · 2021-06-21T09:01:27Z

Thanks @nstarman !

nstarman · 2021-06-21T16:37:23Z

Thanks for the reviews!
Now to get this into Astropy. But I guess that will require a new release of sphinx-astropy? Or is there a nightly release or something?

pllim · 2021-06-21T17:11:16Z

Re: release -- Probably needs a new release but... @saimn , looks like we never set up a auto publish workflow here. Hmm!

saimn · 2021-06-21T21:11:52Z

Yep, will try to set it up in the coming days.

pllim · 2021-06-22T20:52:48Z

@nstarman , this is now in https://pypi.org/project/sphinx-astropy/1.4/

nstarman · 2021-06-22T21:21:00Z

Awesome! Thanks.
Now I can get this in for Astropy main.

pllim · 2021-06-22T21:39:44Z

Many thanks to @saimn ! 😸

nstarman force-pushed the intersphinx-xref-physical-types branch from a0b5de7 to 557cf89 Compare May 1, 2021 23:50

nstarman commented May 1, 2021

View reviewed changes

sphinx_astropy/conf/v1.py Outdated Show resolved Hide resolved

sphinx_astropy/conf/v1.py Outdated Show resolved Hide resolved

sphinx_astropy/conf/v1.py Outdated Show resolved Hide resolved

nstarman changed the title ~~intersphinx xref physical types~~ intersphinx xref May 2, 2021

nstarman commented May 2, 2021

View reviewed changes

setup.cfg Show resolved Hide resolved

nstarman commented May 2, 2021

View reviewed changes

sphinx_astropy/conf/v1.py Outdated Show resolved Hide resolved

nstarman mentioned this pull request May 2, 2021

Use sphinx-astropy xref aliases astropy/astropy#11684

Merged

3 tasks

nstarman commented May 2, 2021

View reviewed changes

sphinx_astropy/conf/v1.py Show resolved Hide resolved

nstarman commented May 2, 2021

View reviewed changes

sphinx_astropy/conf/v1.py Outdated Show resolved Hide resolved

nstarman mentioned this pull request May 2, 2021

Tie intersphinx inventory locations to versions being installed #15

Open

nstarman force-pushed the intersphinx-xref-physical-types branch from bc055c7 to 79f4303 Compare May 24, 2021 22:25

astrofrog requested changes May 26, 2021

View reviewed changes

sphinx_astropy/conf/v1.py Outdated Show resolved Hide resolved

nstarman requested a review from astrofrog May 27, 2021 02:16

pllim requested a review from saimn June 14, 2021 17:21

saimn reviewed Jun 14, 2021

View reviewed changes

saimn approved these changes Jun 15, 2021

View reviewed changes

nstarman added 7 commits June 17, 2021 12:23

intersphinx xref physical types

2d70631

Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>

astropy version protect

86d9015

Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>

Seed some python terms to numpy_xref_aliases

4095608

Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>

add some of astropy’s glossary

c1cb5b7

Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>

combined alias

f0805eb

Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>

make Astropy an optional dependency

c531506

cleanup dynamic generation Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>

use astropy for minversion check

1001d69

The version check only runs if astropy is installed. Also update numpydoc xref ignores, from review comment. Signed-off-by: Nathaniel Starkman (@nstarman) <[email protected]>

nstarman force-pushed the intersphinx-xref-physical-types branch from d57a237 to 1001d69 Compare June 17, 2021 16:27

saimn merged commit 3703265 into astropy:main Jun 21, 2021

nstarman deleted the intersphinx-xref-physical-types branch June 21, 2021 16:35

pllim mentioned this pull request Jun 22, 2021

Release v1.4 #43

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

intersphinx xref #40

intersphinx xref #40

nstarman commented May 1, 2021 •

edited

Loading

nstarman commented May 2, 2021 •

edited

Loading

saimn commented May 4, 2021

nstarman commented May 4, 2021 •

edited

Loading

nstarman commented May 25, 2021 •

edited

Loading

astrofrog left a comment

nstarman commented Jun 1, 2021 •

edited

Loading

saimn left a comment

saimn Jun 14, 2021

nstarman Jun 15, 2021

nstarman Jun 15, 2021

saimn left a comment

nstarman commented Jun 17, 2021 •

edited

Loading

saimn commented Jun 21, 2021

nstarman commented Jun 21, 2021

pllim commented Jun 21, 2021

saimn commented Jun 21, 2021

pllim commented Jun 22, 2021

nstarman commented Jun 22, 2021

pllim commented Jun 22, 2021

intersphinx xref #40

intersphinx xref #40

Conversation

nstarman commented May 1, 2021 • edited Loading

nstarman commented May 2, 2021 • edited Loading

saimn commented May 4, 2021

nstarman commented May 4, 2021 • edited Loading

nstarman commented May 25, 2021 • edited Loading

astrofrog left a comment

Choose a reason for hiding this comment

nstarman commented Jun 1, 2021 • edited Loading

saimn left a comment

Choose a reason for hiding this comment

saimn Jun 14, 2021

Choose a reason for hiding this comment

nstarman Jun 15, 2021

Choose a reason for hiding this comment

nstarman Jun 15, 2021

Choose a reason for hiding this comment

saimn left a comment

Choose a reason for hiding this comment

nstarman commented Jun 17, 2021 • edited Loading

saimn commented Jun 21, 2021

nstarman commented Jun 21, 2021

pllim commented Jun 21, 2021

saimn commented Jun 21, 2021

pllim commented Jun 22, 2021

nstarman commented Jun 22, 2021

pllim commented Jun 22, 2021

nstarman commented May 1, 2021 •

edited

Loading

nstarman commented May 2, 2021 •

edited

Loading

nstarman commented May 4, 2021 •

edited

Loading

nstarman commented May 25, 2021 •

edited

Loading

nstarman commented Jun 1, 2021 •

edited

Loading

nstarman commented Jun 17, 2021 •

edited

Loading