Skip to content

Preserve custom CHECKSUM and DATASUM comments on flush in update mode - #20825

Open
pranjal9091 wants to merge 2 commits into
astropy:mainfrom
pranjal9091:fix/fits-checksum-when-comment
Open

Preserve custom CHECKSUM and DATASUM comments on flush in update mode#20825
pranjal9091 wants to merge 2 commits into
astropy:mainfrom
pranjal9091:fix/fits-checksum-when-comment

Conversation

@pranjal9091

@pranjal9091 pranjal9091 commented Nov 11, 2026 •
edited
Loading

Copy link
Copy Markdown

Description

When a FITS file with existing CHECKSUM and DATASUM cards is modified in update mode, _update_checksum() previously called self.add_checksum() with no arguments. This caused when to default to None, silently replacing any custom comments on the cards with Astropy's default timestamp string ("HDU checksum updated ..." / "data unit checksum updated ...").

This PR:

  1. Updates _update_checksum() to preserve existing custom comments on CHECKSUM and DATASUM cards (comments that do not start with the default timestamp prefix). Default timestamp comments continue to be refreshed with the current timestamp as before.
  2. Updates add_checksum() and add_datasum() to record and preserve user-supplied when values, and adds an optional datasum_when parameter to add_checksum() allowing independent comments on DATASUM and CHECKSUM.
  3. Adds regression tests in test_checksum.py covering both explicit add_checksum(when=...) calls and automatic checksum recalculation on modified data in update mode.
  4. Adds a changelog fragment in docs/changes/io.fits/20825.bugfix.rst.

Fixes #20789

AI Disclosure

I used an AI assistant while investigating the codebase and drafting the initial test structure. I have reviewed, adapted, and verified all changes and tests locally.

  • I certify that I am human and that I take full responsibility for this pull request including all interactions with reviewers.

@pranjal9091
pranjal9091 requested a review from saimn as a code owner November 11, 2026 05:27
@giles

giles Bot commented Nov 11, 2026

Copy link
Copy Markdown

Hi 👋 and thank you for your contribution! 🙏

This pull request is being closed automatically - but don't worry, we may re-open it if you carefully follow these instructions.

Join the astropy slack using this invite link, head over to the #new-contributor-requests channel, and introduce yourself with a post that includes:

  1. Background about your current occupation or affiliation.

  2. How the bug or missing feature affects your work.

We strongly encourage contributions from people that use astropy or actively develop related scientific Python packages. We look forward to hearing from you and growing the Astropy community!

Notes for maintainers

If you know this is a genuine user and the PR is is compliance with the AI policy, you may re-open this straight away. After two successful merged pull requests, the user will be added to the allowlist and future PRs will no longer be closed automatically.

This user has not made any pull requests to this repository prior to this one.

In addition, here are some statistics on the user's activity on GitHub:

Last 24 hours Last 7 days
Pull requests opened 3 3
Issues opened 0 0

@giles giles Bot closed this Nov 11, 2026
@github-actions

github-actions Bot commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

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.

@pranjal9091

pranjal9091 commented Nov 11, 2026

Copy link
Copy Markdown
Author

Hi @saimn, I noticed that giles closed this automatically because I am a first-time contributor to Astropy.

This PR fixes #20789 by preserving custom CHECKSUM and DATASUM comments on flush in update mode. It includes unit tests in test_checksum.py and a changelog fragment in docs/changes/io.fits/20825.bugfix.rst, all passing pre-commit and local test suites.

Could you please take a look and reopen when you have a chance? Thank you!

@pllim pllim reopened this Nov 11, 2026
@pllim
pllim requested a review from tepickering November 11, 2026 16:01
@pllim pllim added this to the v8.1.0 milestone Nov 11, 2026
@pllim pllim added the Bug label Nov 11, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@saimn saimn Awaiting requested review from saimn saimn is a code owner
@tepickering tepickering Awaiting requested review from tepickering

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Bug io.fits

Projects

None yet

Milestone

v8.1.0

Development

Successfully merging this pull request may close these issues.

FITS checksum comments set by add_checksum(when=...) are overwritten on flush in update mode

2 participants

@pranjal9091 @pllim