Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

• Do the proposed changes actually accomplish desired goals?
• Do the proposed changes follow the Astropy coding guidelines?
• Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
• Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
• Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
• Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
• Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
• Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
• Is a milestone set? Milestone must be set but we cannot check for it on Actions; do not let the green checkmark fool you.
• At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

👋 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?

I'm debating whether to define a TypeAlias for kind: Literal["comoving", "lookback", "luminosity"].
Maybe

DistanceKind: TypeAlias = Literal["comoving", "lookback", "luminosity"]

Sigh. RTD errors are the most annoying to resolve. Who the heck knows how it finds its reference targets.

As a note, I'm pretty sure that if we adopt astropy/astropy-APEs#85 then we won't have this problem since I think RTD correctly recognizes _-prefixed modules as private and won't then find duplicates, as is happening here. Not to harp, but standard problems are solved by standard solutions!

WARNING: more than one target found for cross-reference 'Equivalency': astropy.units.Equivalency, astropy.units.equivalencies.Equivalency

I'm excited to see this PR!

Sigh. RTD errors are the most annoying to resolve. Who the heck knows how it finds it's reference targets.

Sigh, indeed. We added a fairly long section on how to deal with Reference target not found RTD errors to PlasmaPy's documentation guide.

Thanks @namurphy. That's a useful guide! Unfortunately I don't think it covers this edge case (see sphinx-doc/sphinx#10400)

937f0af shows that removing Equivalency from astropy.units.equivalencies.__all__ fixes the issue since it's no longer doubly defined.
08d3e40 shows that we can have __all__ defined if the module is properly private.
This problem might arise here and hasn't come up elsewhere because units.__init__.py doesn't have an __all__ defined. Adding __all__ might fix this problem due to sphinx treating it with higher precedence or something. I'll investigate.

Hopefully #15862 will fix this. Sphinx uses the PEP8 definitions of public API, which is how my type annotation tripped it up here. Sphinx also has a precedence criteria, so hopefully defining units.__all__ should work.

Update: unfortunately #15862 is not a fix. The precedence for units.__all__ above units.equivalencies.__all__ didn't work.

Ping @mhvk. I'm trying to resolve an issue with RTD. Sphinx is trying to resolve the type annotations and finds Equivalency at both astropy.units.Equivalency and astropy.units.equivalencies.Equivalency. Sphinx uses PEP8-style rules for determining which to point to. In experiments (08d3e40) I renamed the equivalences module to _equivalencies and this sorted out the problem. I'm looking for a solution we can use short of renaming modules.

One issue we had before with sphinx is that we displayed the definitions of things more than two times (e.g., something like units.function.core, units.function, and units.function). At the time, the sphinx people told us that they can deal with having both a place where things are defined and one where they are exposed, but not also an API link to something in between (if my memory serves, this is why units.function is not shown any more).

I suspect you are running into something similar. To test this, could you try removing astro.units.equivalencies from astropy/docs/units/ref_api.rst and see if that solves things?

I think even if it does not, since sphinx can deal with our setup for the regular documentation, this looks to me like a sphinx bug that they would likely want to know about!

That said, there is something somewhat illogical more generally about showing API/contents for what nominally are private modules. If we could adjust automodapi to insert some headings in between groupings, that might in fact be much nicer (they are actually ordered like that in the current docs, see https://docs.astropy.org/en/latest/units/ref_api.html#module-astropy.units -- but note that this is actually a different order from stable, where it is alphabetical... separate issue -- #15877).

@mhvk. That appears to work! Looks like we also need to remove astropy.units.quantity

OK, great, that at least narrows it down. It sounds like something to raise over at sphinx (independently of whether we change our docs around ourselves).

Given the addition of units.all, I don't think we're losing anything except logical groupings from the docs by removing these references. Should we merge this in and work on the docs in a separate PR?

That seems the wrong order - let's separate out the removals to a new PR that also gets the docs OK again first.

See #15891

Thanks @mhvk, @eerovaher, @eerovaher for the reviews! These RTD errors can be hard to diagnose and fix.

Thanks, @nstarman!

Did this break RTD?

WARNING: py:class reference target not found: astropy.units.quantity.Quantity [ref.class]

build finished with problems, 1563 warnings.

How weird!!

I think this might just need another addition to nitpick-exceptions. We've had to clobber every problem with Sphinx by just ignoring it using this file.