ENH: Move _get_converter to public API

[MridulS]

Description

This PR fixes #9080, fixes #8273, fixes #11206. It almost just reverts the 9 year old commit 55022e9 😄

With this in the public API something like this would be supported behaviour:

In [1]: import astropy.units as u

In [2]: c1 = u.cm.get_converter(u.m)

In [3]: c2 = u.m.get_converter(u.cm)

In [4]: c1(c2(10.0))
Out[4]: 10.0

In [5]: c2(c1(10.0))
Out[5]: 10.0

This does speed up the examples in the issues.

• By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

[github-actions]

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?
• 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.

[MridulS]

Currently FunctionUnitBase does not expose the get_converter method, and I guess they should for consistency. u.Jy.get_converter(u.ABmag) works but u.ABmag.get_converter(u.Jy) doesn't.

If we do end up going forward with this PR should I add the functionality in this PR or create a new one?

[mhvk]

Good point. Essentially, the logic is all in to(), so that would need to be changed. My sense is that you have a nice & easy PR here, so do this as follow-up!

[MridulS]

Perfect! I will create a new PR for that change.

[mhvk]

thanks! This looks good - really just nitpicks about the new docstring...

Also, since this is now public API, we probably should have some explicit tests. I wonder if there were some when we reverted it... If not, I'd suggest the very simple one you have as an example, as well as one involving equivalencies (say, u.parallax()) and one going to ABmag. They may be most logical in different test files...

[mhvk]

Good point. Essentially, the logic is all in to(), so that would need to be changed. My sense is that you have a nice & easy PR here, so do this as follow-up!

[pllim]

Since you pointed it out above, it does revert something that was intentional:

• Deprecate and privatize Unit.get_converter #3474

That was put in to address this problem:

• units are not updated after a conversion #3456

So, you have to make sure the very old Issue 3456 is not resurfacing here. Thanks!

[mhvk]

Good suggestion! @MridulS - I just read through the original issue and there were some specific wording changes suggested there.

[mhvk]

Looks good! There are readthedocs failures, though, see in line comment. Also a suggestion for a bit more explanation in the changelog fragment.

[mhvk]

OK, all good now, let's get this in!