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 error when a PrimaryHDU has a "GROUPS = 1" keyword #14998

Merged
merged 1 commit into from Jul 6, 2023

Conversation

saimn
Copy link
Contributor

@saimn saimn commented Jun 29, 2023

GROUPS = 1 was interpreted as GROUPS = T which is wrong.
This occurs on some Euclid commissioning file so I would like to make a bugfix release soon (with also the nddata fix).

In [1]: %astropy
Numpy 1.25.0
Astropy 6.0.dev302+g6fd7ebdfc

In [2]: hdu = fits.PrimaryHDU()

In [3]: hdu.header["GROUPS"] = (1, "not a groups HDU")

In [4]: hdu.header
Out[4]: 
SIMPLE  =                    T / conforms to FITS standard                      
BITPIX  =                    8 / array data type                                
NAXIS   =                    0 / number of array dimensions                     
EXTEND  =                    T                                                  
GROUPS  =                    1 / not a groups HDU                               

In [5]: hdu.writeto("test.fits")

In [6]: fits.open("test.fits")
WARNING: An exception occurred matching an HDU header to the appropriate HDU type:  [astropy.io.fits.hdu.base]
WARNING: The HDU will be treated as corrupted. [astropy.io.fits.hdu.base]
Out[6]: [<astropy.io.fits.hdu.base._CorruptedHDU object at 0x7f5640149f10>]

@saimn saimn added io.fits 💤 backport-v5.0.x on-merge: backport to v5.0.x 💤 backport-v5.3.x on-merge: backport to v5.3.x labels Jun 29, 2023
@saimn saimn added this to the v5.0.7 milestone Jun 29, 2023
@saimn saimn requested a review from astrofrog June 29, 2023 16:55
@github-actions
Copy link

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 "When to rebase and squash commits".
  • 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?
  • Is a milestone set? Milestone must be set but we cannot check for it on Actions; do not let the green checkmark fool you.
  • 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.

raise InvalidHDUException
else:
return False
return card.keyword == "SIMPLE" and card.value is False
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is not strictly needed since _NonstandardHDU comes after the other main extensions when finding the correct HDU class, but as the comment says this check is useless.

Copy link
Member

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

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

Checked the standard and this looks right, thanks!

@astrofrog astrofrog merged commit 1d3b81a into astropy:main Jul 6, 2023
24 of 26 checks passed
@lumberbot-app

This comment was marked as resolved.

meeseeksmachine pushed a commit to meeseeksmachine/astropy that referenced this pull request Jul 6, 2023
@saimn saimn deleted the fits-fix-groups branch July 6, 2023 13:36
saimn added a commit that referenced this pull request Jul 6, 2023
…998-on-v5.3.x

Backport PR #14998 on branch v5.3.x (Fix error when a PrimaryHDU has a "GROUPS = 1" keyword)
saimn pushed a commit to saimn/astropy that referenced this pull request Jul 6, 2023
saimn added a commit that referenced this pull request Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io.fits 💤 backport-v5.0.x on-merge: backport to v5.0.x 💤 backport-v5.3.x on-merge: backport to v5.3.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants