Don't log when passed unit matches FITS BUNIT by m4cd4r4 · Pull Request #19389 · astropy/astropy

m4cd4r4 · 2026-03-10T04:48:14Z

Description

Don't log when passed unit matches FITS BUNIT

CCDData.read logs an info message whenever a unit argument is passed alongside a FITS file that has a BUNIT header, even when they match. This is noisy when users explicitly pass the unit to confirm what's already in the file.

The log now only fires when the two units actually differ.

Fixes #13539

By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

github-actions · 2026-03-10T04:48:26Z

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.

pllim · 2026-03-10T15:16:23Z

Probably need regression test.

m4cd4r4 · 2026-03-20T10:52:25Z

Hi @pllim - the regression test was added in commit 9a80dda (March 12). It covers the case where the passed unit matches FITS BUNIT and verifies no log message is emitted. All CI is green. Happy to make any adjustments if needed.

mwcraig

Thanks for opening this, @m4cd4r4. Just some minor changes...

mwcraig · 2026-03-23T20:04:32Z

+                    fits_unit_parsed = u.Unit(fits_unit_string)
+                except ValueError:
+                    fits_unit_parsed = None
+                if fits_unit_parsed is None or fits_unit_parsed != u.Unit(unit):


I would prefer a comparison of the string units as in #18678 rather than trying to parse the unit. There are some "valid" fit unit strings that do not immediately translate into astropy units.

m4cd4r4 · 2026-03-24T08:33:42Z

Done - switched to str(unit) \!= str(fits_unit_string) to match the approach in #18678. Removed the try/except block and unit parsing entirely.

mwcraig · 2026-03-25T14:16:56Z

Wonderful, thanks @m4cd4r4! Two more quick things:

Can you please rebase? Looks like this is out of date with main.
Could you please add @Warnocke as a co-author on the last commit? That won't change who gets credit for this PR but will acknowledge their earlier work.

Co-authored-by: Evan Warnock <176846909+Warnocke@users.noreply.github.com>

m4cd4r4 · 2026-03-25T22:47:37Z

Done - rebased onto main and added @Warnocke as co-author on the last commit.

m4cd4r4 · 2026-04-08T05:20:44Z

Friendly ping @mwcraig - this has been ready for re-review since 2026-03-25 (rebase + co-author both done). No rush, just flagging in case it slipped through. Happy to make any further adjustments.

mwcraig

Thanks for the ping @m4cd4r4 -- this just needs a rebase and it is ready to merge

mwcraig · 2026-04-08T13:46:44Z

Actually, I take that back -- no rebase is needed

pllim

Not sure why you used \

pllim · 2026-04-08T14:11:48Z

@@ -0,0 +1 @@
+Fixed \CCDData.read\ logging a spurious info message when the unit passed by the caller matches the \BUNIT\ value in the FITS header.


Suggested change

Fixed \CCDData.read\ logging a spurious info message when the unit passed by the caller matches the \BUNIT\ value in the FITS header.

Fixed ``CCDData.read()`` logging a spurious info message when the unit passed by the caller matches the BUNIT value in the FITS header.

Applied in 6e15ec5 - switched to RST double backticks. Thanks for catching that.

github-actions bot added the nddata label Mar 10, 2026

pllim added this to the v8.0.0 milestone Mar 10, 2026

mwcraig requested changes Mar 23, 2026

View reviewed changes

m4cd4r4 and others added 6 commits March 26, 2026 06:47

Don't log when passed unit matches FITS BUNIT

ee1e47d

Add changelog entry for astropy#13539

2f07e8e

Rename changelog fragment to match PR number

844aa5f

Remove incorrectly named changelog fragment

b1d09db

Add regression test for astropy#13539 - no log when unit matches BUNIT

da28ff2

Use string comparison for unit vs BUNIT in fits_ccddata_reader

f72e2f8

Co-authored-by: Evan Warnock <176846909+Warnocke@users.noreply.github.com>

m4cd4r4 force-pushed the fix/ccddata-redundant-log branch from 34c476d to f72e2f8 Compare March 25, 2026 22:47

mwcraig approved these changes Apr 8, 2026

View reviewed changes

pllim requested changes Apr 8, 2026

View reviewed changes

Use RST double backticks in changelog entry

6e15ec5

mwcraig requested a review from pllim April 16, 2026 14:07

		@@ -0,0 +1 @@
		Fixed \CCDData.read\ logging a spurious info message when the unit passed by the caller matches the \BUNIT\ value in the FITS header.

	Fixed \CCDData.read\ logging a spurious info message when the unit passed by the caller matches the \BUNIT\ value in the FITS header.
	Fixed ``CCDData.read()`` logging a spurious info message when the unit passed by the caller matches the BUNIT value in the FITS header.

Uh oh!

Conversation

m4cd4r4 commented Mar 10, 2026

Description

Description

Uh oh!

github-actions bot commented Mar 10, 2026

Uh oh!

pllim commented Mar 10, 2026

Uh oh!

m4cd4r4 commented Mar 20, 2026

Uh oh!

mwcraig left a comment

Choose a reason for hiding this comment

Uh oh!

mwcraig Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

m4cd4r4 commented Mar 24, 2026

Uh oh!

mwcraig commented Mar 25, 2026

Uh oh!

m4cd4r4 commented Mar 25, 2026

Uh oh!

m4cd4r4 commented Apr 8, 2026

Uh oh!

mwcraig left a comment

Choose a reason for hiding this comment

Uh oh!

mwcraig commented Apr 8, 2026

Uh oh!

pllim left a comment

Choose a reason for hiding this comment

Uh oh!

pllim Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

m4cd4r4 Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants