Return NaN for out of bound conversions in FITS WCS

[astrofrog]

I updated the APE 14 base class to say 'should' instead of 'can' because I think it's important we have consistent behavior.

[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.

[github-actions]

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

[pllim]

From the test failures, this looks like a high impact change?

Does https://github.com/astropy/astropy-APEs/blob/main/APE14.rst need to be updated along with this PR?

[pllim]

I don't think this should be backported.

[astrofrog]

@pllim - we don't need to, the APE says 'We note that while we have made efforts to ensure that the API described here is as close as possible to the final implemented API, the authoritative version of the API will be given by the base classes that live in the core astropy package.'

Agree about not backporting.

[nden]

By default GWCS sets out-of-bounds to NaN. However, it's very useful in certain cases to be able to evaluate the transforms without limits. I think it's a shortcoming of APE 14 that it doesn't allow passing kwargs. I understand why this is so but perhaps certain standard keywords can be allowed and an application can set them.

Another difference with GWCS is that while pixel_bounds don't have a setter, it is possible to reset them to None because they are tied to a property bounding_box which defines the bounds. Setting the bounding_box to None essentially sets pixel_bounds to None. Being able to reset the bounds allows flexibility which is perhaps not necessary for applications but quite useful for interactive work.

astropy.modeling expects numerical intervals for all directions, we'll have to add an option of None at some point to match this behaviour, and that's fine.

The failing tests are all in SlicedWCS, that's not too bad and is perhaps to be expected.
And finally, since I've been advocating for allowing this behavior from the start, I'd consider this a bug, not a new feature :-). But I agree with whatever way is easier/best to handle it.

[astrofrog]

I've now fixed the tests - the main failing tests were those for SlicedLowLevelWCS which had unrealistic pixel_bounds which didn't even include the CRPIX values. I've now updated this.

In practice, this should not impact many users because they would have needed to explicitly set pixel_bounds on a FITS WCS - it is never populated on reading from a FITS file.

[astrofrog]

Even though it's technically a bug, as it's an API change I think we can just go with 7.0.0

[pllim]

I think it needs a rebase to pick up updated test matrix.

[astrofrog]

Rebased!