Add a new keyword argument negative_lon_cdelt to find_optimal_celestial_wcs

[astrofrog]

This defaults to None which means auto. This is a break in API compared to before since for solar WCS for example, cdelt will now be positive. I'm not sure how to change the default via a deprecation phase though, so maybe we have to just bite the bullet?

Fixes #431

@svank @Cadair - any opinions?

[Cadair]

Seems sane, though I don't know how complete the list of negative frames is?

[astrofrog]

Well I tried to base is on the list of valid celestial frames from WCS paper II, I need to investigate whether there are others that could be possible.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.89%. Comparing base (ce6538c) to head (fd0c829).
Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #528      +/-   ##
==========================================
+ Coverage   88.86%   88.89%   +0.03%     
==========================================
  Files          28       28              
  Lines        1374     1378       +4     
==========================================
+ Hits         1221     1225       +4     
  Misses        153      153              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[astrofrog]

Well as far as I can tell I think that's it, I don't think there are other standardized ones. I've decided to update the docstring to spell out the automation rule, so it's not some secret implementation detail.

[Cadair]

Is there supposed to be an import here. Did pre-commit steal it?

[svank]

Suggested change

	negative_lon_cdelt : bool, optional
	Whether the CDELT value for the longitude coordinate should be negative
	(`True`) or positive (`False`). For astronomical observations of the
	sky this is usually the case, while for coordinate systems used in
	solar physics this is usually positive. If this is not specified, the
	value will be `True` if the first four characters for CTYPE for the
	longitude is ``RA--``, ``GLON``, ``ELON``, ``HLON``, or ``SLON``, and
	`False` otherwise.
	negative_lon_cdelt : bool, optional
	Whether the CDELT value for the longitude coordinate should be negative
	(`True`) or positive (`False`). For astronomical observations of the
	sky it is usually negative, while for coordinate systems used in
	solar physics this is usually positive. If this is not specified, the
	value will be `True` if the first four characters for CTYPE for the
	longitude is ``RA--``, ``GLON``, ``ELON``, ``HLON``, or ``SLON``, and
	`False` otherwise.

I think this reads more clearly

[svank]

The only way I can think of to have a deprecation period is to trigger the 'auto' behavior when negative_lon_cdelt='auto', and set the default to None for a release, which is the auto behavior + a warning.

One other thought is that this PR does negative CDELT for only a whitelist of frames, which, since having it be negative feels like the "weird case" in general, it seems right to only do it when we know it's right to do so. But if instead there were a list of known "positive CDELT" frames and otherwise negative CDELT were the default, then we'd only be changing the function's behavior when we know that's right to do so. As-is feels better long-term, but flipping the sense of the whitelist feels less disruptive short-term. Thoughts?

[astrofrog]

Well to put it on the table we could also just keep the default as True and make False or auto opt in.