Repository navigation
Record data size in the WCS object #5461
Description
It is clear that there are practical reasons to have a record of the original data size in the WCS object. This in the past has been recorded in public attributes naxis1 and naxis2 and subsequently in the private _naxis1 and _naxis2. There's along thread on why this should or should not be done in #4662.
More recently #5411 expanded this attribute to more than 2 axes. It also broke the ability to set the private attributes which was subsequently fixed in #5454 as a stop gap measure.
We need to fix this permanently starting with v 1.3. There are two proposed ways to do it.
- Add public
naxisjattributes, populated by the FITS headerNAXISjkeywords. - Introduce a subclass of
WCSwhich adds those and leave the original WCS to represent strictly the FITS WCS standard and nothing more.
Either way a deprecation process will accompany the change.
The purpose of this issue is to collect votes. I don't want to start a new long discussion. Keep in mind that #4662 is available for reference and only share new arguments.
Since 1.3 is approaching we need to make a fast decision. So please share your preference here and volunteer for the work if possible.
Activity
nden commentedon Nov 4, 2016
I support #1 of the two choices for reasons I mentioned in #5454.
In addition I want to mention that we already deviated from the FITS standard by adding the
d2imdistortion.saimn commentedon Nov 7, 2016
👍 for solution 1, for the reasons explained by @nden in #5411 (comment)
MSeifert04 commentedon Nov 13, 2016
hm, I'm sure that #4662 does not contain the extended discussion.
mcara commentedon Nov 24, 2016
@MSeifert04 I think the "long discussion" was in #4669
mcara commentedon Nov 24, 2016
👍 for solution 1
mwcraig commentedon Nov 28, 2016
Also 👍 for solution 1. Looking forward to having those keywords update when I slice...
astrofrog commentedon Nov 29, 2016
With apologies to @nden (who asked to not start a discussion), I want to make a case for option 2 (following discussion on e.g. #5455). But I'll keep it short.
WCS objects allow slicing, including by floating-point values. For instance, I can do:
I can see the motivation for this and it looks like people are relying on this. However, this causes issues when the image size is present in the WCS because it's no longer possible to necessarily scale the image size to an integer size. So the presence or not of image dimensions changes how slicing works.
Furthermore, another example where the presence/absence of image shape matters is negative indices. If I do
wcs[::-1,::-1], this can only work if I have an image size.Because the slicing behaves so differently between the two cases, I'm 👍 on option 2 because it will allow the behavior to be more predictable and separates the two different use cases.
Note: I didn't say that
WCShas to be the 'pure' class though.WCScould be the class with image shape if there is a superclass that does not (we'd just need to find a good name). So just to be clear, there's a difference between separating the classes versus which one should be the 'default'.nden commentedon Dec 1, 2016
@astrofrog My main concern is giving an option to users to choose between two classes when they want to implement a WCS pipeline for an instrument. How would I choose which one to subclass?
astrofrog commentedon Dec 1, 2016
@nden - I see your point. It seems I'm in the minority anyway, so I'd say just go with the majority opinion rather than try and get everyone to agree :)
nden commentedon Jun 12, 2017
It'd be good to coordinate the interface for this with the interface used in GWCS.