Rename AltAz -> Horizontal - #7708

Hi there @adrn 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃.

I noticed the following issue with this pull request:

• The milestone has not been set (this can only be set by a maintainer)

Would it be possible to fix this? Thanks!

If there are any issues with this message, please report them here.

Whoops, I guess I need to define the transformations for the deprecated class as well - this is currently broken.

OK, the last commit re-defines transformations for the deprecated class, and adds a test that transformations still work for AltAz. If this gets merged, we should open an issue to remind us to remove the various TODO items added here, surrounding the deprecation.

I vote for solving #6533 with a documentation fix as proposed by @astrofrog instead of an API change. Like many production users of astropy, I rely on stability and so am 👎 on top-level API changes like this unless there is a really strong driver.

I also find AltAz to be a much more obvious and commonplace name and I have never heard Horizontal actually used in the wild.

I think I agree with @taldcroft here - it is not that confusing given that in every class the longitude is first.

OK, I don't have a strong opinion, but I think something should be done because it is confusingly named.

I've always found the docstrings for the frame classes a bit confusing (since there should be "or"'s in the argument specification, i.e. in this case alt, az, distance OR representation). But I don't immediately see a way to clean this up and still be consistent with the numpydoc style. I don't see any real convergence or suggestions to change the documentation in #6533...

I've also always wanted the call signature for each frame (that appears with IPython tab complete or ?) to have the argument names, but this faces the same issue (right now they all look like, e.g., Init signature: ICRS(*args, ...kwargs) instead of Init signature: ICRS(ra, dec, distance, pm_ra_cosdec, pm_dec, radial_velocity, ...kwargs)).

I've always found the docstrings for the frame classes a bit confusing (since there should be "or"'s in the argument specification, i.e. in this case alt, az, distance OR representation).

This sounds like a pretty strong case for not providing this functionality in a single constructor but providing alternative constructors like AltAz.from_represantation

We're somewhat stuck with allowing representation as an input, but I guess that doesn't mean we cannot also have the classmethod and in the docstring focus on the most natural way to initialize; I certainly agree that right now it is somewhat confusing. That said, most people probably initialize SkyCoord and there it is a bit less obvious what to do...

I've also always wanted the call signature for each frame (that appears with IPython tab complete or ?) to have the argument names, but this faces the same issue (right now they all look like, e.g., Init signature: ICRS(*args, ...kwargs) instead of Init signature: ICRS(ra, dec, distance, pm_ra_cosdec, pm_dec, radial_velocity, ...kwargs)).

Please open an issue for this as a reminder, this is something that also bugs me for a long time, and probably (maybe) there are solutions/workaround for it.

@taldcroft @adrn The term "Horizontal coordinates" is used a lot in radio astronomy. I guess that having some sort of alias would not hurt?

@adrn - I agree with @taldcroft (and also I said it more or less in #6533) that I'm 👎 on changing this name. I know this is in the eye of the beholder (probably depends on the textbook/web page/instruction manual you first read it in or something...), but to me "AltAz" is a more natural name than "Horizontal". (Or "AzAlt", which I've never heard someone say out loud.) I think the order as-implemented actually comes from the fact that RA comes before Dec, which moticated the lon,lat order... but that the opposite of "altaz". So like so many things in astronomy, we can blame archaic historical terminology...

All that said, I'm not averse to @matteobachetti's suggestion of it being an alias. But it won't work to have it be a subclass or something, because then the transforms won't work (and I think it'd be conceptually confusing).

What if we raised a warning if the arguments are passed as positional arguments, recommending that they instead be passed as keyword arguments, to avoid confusion? Then in the long run we could even deprecate the version without keyword arguments? (specifically for AltAz I mean, not all frames).

As a side thought, if I had to guess, I would say the majority of users don't actually initialize AltAz - they initialize other frames and convert to AltAz then access .az and .alt (it's rarer to go in the other direction).

I also agree with @taldcroft and the others above that are 👎 on this change.

What if we raised a warning if the arguments are passed as positional arguments, recommending that they instead be passed as keyword arguments, to avoid confusion? Then in the long run we could even deprecate the version without keyword arguments? (specifically for AltAz I mean, not all frames).

That's less disruptive, but it's still (IMO unnecessary) churn for projects using Astropy. I'm sure a lot of code has been written already in the past 5 years or so using this, and

The choice to call it AltAz might not have been ideal, but it wasn't completely un-motivated, it's common to call it that. E.g. see https://en.wikipedia.org/wiki/Altazimuth_mount or also at https://en.wikipedia.org/wiki/Horizontal_coordinate_system it's called Alt/Az and not Az/Alt, whereas colloquially it's called e.g. Ra/Dec for equatorial.

Suggest to do a docstring addition and an example at http://docs.astropy.org/en/stable/api/astropy.coordinates.AltAz.html
The docstring can give the recommendation to call by keyword arguments especially here to be clear in the code, but I'm 👎 even on starting to issue warnings here.

If Astropy does backward-incompatible changes (like disallowing positional args as discussed as one course of action here), please do it in a major release, not 3.1, which is the current milestone for this issue.

I created 4.0 milestone and assigned this issue to it. FYI.

@pllim - Even for a 4.0 release I don't think there will be much support for an actual API change or warnings for non-kwarg arguments. I personally think that this has gotten a good discussion and broad input, and now we should close this PR in favor of an open issue to update the documentation.

@adrn?

OK, feel free to re-milestone or whatever. I am not following this closely nor have enough expertise to comment further. I am sure the milestone will come in handy for other issues anyway. 😄

I'm removing the 4.0 milestone, I think whatever decision is taken now, we should just stick to it.

Personally, I'm 👎 on this PR and in favor of better docs.

Also 👎

I'll close this...

I think we should probably have an issue about documenting this?