DM-11862: Fixes to PosixStorage read/write #78
Conversation
| if additionalData.exists("flags"): | ||
| kwds = dict(flags=additionalData.getInt("flags")) | ||
| else: | ||
| kwds = {} |
There was a problem hiding this comment.
Do we want another if here for mode="a"? I don't know if there's any way to include "appending" (instead of overwriting) in a butler call.
There was a problem hiding this comment.
so the same thing here: IMO the fix in this ticket should only be a correct factoring of Jim's commit and if you want to improve on the changes he made I'd like to address that in a separate ticket.
There was a problem hiding this comment.
I don't think we never want the butler to modify a dataset in-place. If we did, we'd need to first figure out how that interacts with provenance.
| finalItem = pythonType.readFits(logLoc.locString(), hdu, flags) | ||
| kwds = {} | ||
| if additionalData.exists("hdu"): | ||
| kwds["hdu"] = additionalData.getInt("hdu") |
There was a problem hiding this comment.
This does lose the INT_MIN default value for "hdu", which we should probably keep? There was a ticket on that topic a while back, and I don't recall the results of it...
There was a problem hiding this comment.
In slack you pointed out the breaking commit, it was a76108b#diff-9c0ba521c98745f8b624916b947c6f4dR698.
The new code here is actually the old code from that diff, at line 375.
That code was introduced by @TallJimbo, in commit ce7e05b, which was created after I created the ticket branch for my commit a76108b, and when I rebased my ticket branch onto master before committing it dropped his change.
So as far as losing INT_MIN goes, you'll have to talk to Jim about that. But as far as correcting my factoring, losing the INT_MIN is the right thing to do.
There was a problem hiding this comment.
Yes, we need to drop the hdu argument that defaults to INT_MIN. Not all readFits methods take that argument (including the one on PhotoCalib), so we should only forward it if it's present.
No description provided.