RCAL-723 Fix model.meta.filename when saving model
#295
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #295 +/- ##
==========================================
+ Coverage 97.07% 97.24% +0.17%
==========================================
Files 29 29
Lines 2526 2538 +12
==========================================
+ Hits 2452 2468 +16
+ Misses 74 70 -4 ☔ View full report in Codecov by Sentry. |
| @contextmanager | ||
| def _temporary_update_filename(datamodel, filename): | ||
| """ | ||
| Context manager to temporarily update the filename of a datamodel so that it | ||
| can be saved with that new file name without changing the current model's filename | ||
| """ | ||
| from roman_datamodels.stnode import Filename | ||
|
|
||
| if "meta" in datamodel._instance and "filename" in datamodel._instance.meta: | ||
| old_filename = datamodel._instance.meta.filename | ||
| datamodel._instance.meta.filename = Filename(filename) | ||
|
|
||
| yield | ||
| datamodel._instance.meta.filename = old_filename |
There was a problem hiding this comment.
This specifically exists so that the DataModel instance being saved via save or to_asdf does not update its filename, the filename is only updated in the newly written ASDF file and the model which can be opened from that.
I chose to do it this way because AsdfFile.write_to is employed by these to_asdf (which is called by save), which writes the AsdfFile object to the new file descriptor passed to it, but it does not update the file descriptor of the AsdfFile object. This means the asdf file wrapped by the DataModel instance is not changing, so the filename of the python object being saved should not be updated.
|
Could you add a regression test link? I think this may break them (if previous tests were not careful with file names). |
You are quite right, it breaks most of the regression tests. See https://plwishmaster.stsci.edu:8081/job/RT/job/Roman-Developers-Pull-Requests/502/. In reviewing them, it looks like all the failures are due to the |
|
Let's discuss what we want here at a Thursday tag up. Honestly, it's not always clear to me what one wants for meta.filename. I guess if you save it you want it to be the filename? But if you pass it through steps should the steps be updating it to what they would save it to, were they to write it out? When I was playing with related things last week I got a bit confused about when meta.filename was used to decide what output filename to use, vs. when command line arguments were used, vs. when file names were constructed automatically from the step names. But let's do this after the upcoming release. |
|
@WilliamJamieson Feel free to merge once checks pass |
Resolves RCAL-723
Closes spacetelescope/romancal#1026
When saving a
DataModelto an arbitrary ASDF file, it was noted that themodel.meta.filenamedoes not reflect the name of the file it was saved to. This PR makes sure that when saving aDataModeltofoo.asdfthatDatamodel.meta.filename==foo.asdfin the saved file.Note that this solves RCAL-723 when running individual steps or outputting all the step results.
Checklist
CHANGES.rstunder the corresponding subsection