Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

find_optimal_celestial_wcs does not accept GWCS #328

Closed
pllim opened this issue Dec 23, 2022 · 6 comments · Fixed by #334
Closed

find_optimal_celestial_wcs does not accept GWCS #328

pllim opened this issue Dec 23, 2022 · 6 comments · Fixed by #334
Labels

Comments

@pllim
Copy link
Member

pllim commented Dec 23, 2022

It complains that GWCS has no attribute naxis.

@astrofrog
Copy link
Member

I investigated this a bit and there are two main things to do/consider here:

  • There are a number of places in this function where we assume the input WCS is a FITS WCS, so that will require a bit of thinking. I think we might want to keep the current code for FITS WCS and then write a general APE 14 fallback (I think we should keep the FITS-specific code for backward compatibility and also because there are some things we do smartly if it is a FITS WCS).

  • We currently construct a FITS WCS as the output - I don't think we could easily change this to depend on the input type as it would be a lot of special casing, but I think it would be reasonable to always use FITS WCS as the output if there are no objections?

@pllim
Copy link
Member Author

pllim commented Jan 17, 2023

I think it would be reasonable to always use FITS WCS as the output if there are no objections?

I dunno. GWCS exists to handle distortions that FITS WCS cannot, so if that is not an issue here, then maybe it is okay but I'll ping @nden and @WilliamJamieson to comment.

@astrofrog
Copy link
Member

@pllim - indeed but this is a WCS that the data would get reprojected to, so it's not like the case of raw data when you need GWCS to represent the coordinate system without any resampling. The target WCS doesn't have to have any relation to the original pixel gridding/distortions.

@pllim
Copy link
Member Author

pllim commented Jan 17, 2023

So, if I want to reproject a distorted GWCS to another distorted GWCS, is the distortion ignored? Or is distortion ignored in all cases regardless anyway?

@astrofrog
Copy link
Member

There are two separate things here:

  • reproject can reproject images with/without distortions to WCSes with/without distortions. It will respect the input/output WCS and that means taking into account distortions.

  • The function here is designed to find an 'ideal' output WCS given one or more input WCSes and doesn't include distortions in the returned WCS (because if you are going to reproject/mosaic, why would you want to keep distortions?)

If what you want is to apply a pure rotation to an existing WCS and preserve distortions etc then it's best to just edit the WCS, eg in GWCS you would add a rotation component. Does this make sense?

@pllim
Copy link
Member Author

pllim commented Jan 18, 2023

Maybe... but it really depends on what @orifox wanted to do with this function. I used this because of the code he provided for Imviz rotation work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants