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

Fix AIPS velocity ctype headers for correct rendering #1190

Merged
merged 3 commits into from
Sep 2, 2022

Conversation

pford
Copy link
Collaborator

@pford pford commented Sep 1, 2022

Reverts to previous behavior using casacore::ImageHeaderToFITS for extended file info when the AIPS velocity headers are encountered.

  • What is fixed?
    Frontend issue #1771

  • How does this PR solve the issue? Give a brief summary.
    CFITSIO is used to read the headers as usual. If CTYPE header value FELO-HEL or VELO-LSR is detected, the previous behavior using casacore to generate headers from the FITSImage is used so that image is rendered correctly in the frontend.

  • Are there any companion PRs (frontend, protobuf)?
    No

  • Is there anything else that testers should know (e.g. exactly how to reproduce the issue)?
    FITS images and screenshots are attached in frontend issue. The expected behavior described in the issue is now restored.

Checklist

  • changelog updated
  • e2e test passing / added corresponding fix
  • no protobuf update needed
  • added reviewers and assignee
  • added ZenHub estimate, milestone, and release

@pford pford added this to the v4.0-b1 milestone Sep 1, 2022
Copy link
Contributor

@kswang1029 kswang1029 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all current e2e tests are passing. Manual tests are fine as far as I can tell. This would widen the support of FITS image cubes.

@kswang1029
Copy link
Contributor

@ajm-asiaa is it possible to make an AppImage for @jehibbard to test with?

@ajm-ska
Copy link
Collaborator

ajm-ska commented Sep 2, 2022

@kswang1029 @jehibbard OK. This x86_64 Linux AppImage has been built with the pam/aips_velo_axis backend branch:

wget https://carta.asiaa.sinica.edu.tw/nightly/carta-22.09.02-72a7a1b-77afdaa6-x86_64-AppImage.tgz
tar -xvf carta-22.09.02-72a7a1b-77afdaa6-x86_64-AppImage.tgz 
./carta-22.09.02-72a7a1b-77afdaa6-x86_64.AppImage 

@confluence confluence self-requested a review September 2, 2022 09:24
@confluence confluence merged commit 29e880e into dev Sep 2, 2022
@confluence confluence deleted the pam/aips_velo_axis branch September 2, 2022 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants