From eb54480727514406020e4463cb3fc382e0c0947f Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Fri, 24 Nov 2023 12:39:50 -0500 Subject: [PATCH 1/5] Refactor DataModel.save to use pathlib --- src/roman_datamodels/datamodels/_core.py | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/roman_datamodels/datamodels/_core.py b/src/roman_datamodels/datamodels/_core.py index 53e081e8..031fd476 100644 --- a/src/roman_datamodels/datamodels/_core.py +++ b/src/roman_datamodels/datamodels/_core.py @@ -11,10 +11,8 @@ import copy import datetime import functools -import os -import os.path import sys -from pathlib import PurePath +from pathlib import Path, PurePath import asdf import numpy as np @@ -181,17 +179,9 @@ def clone(target, source, deepcopy=False, memo=None): target._ctx = target def save(self, path, dir_path=None, *args, **kwargs): - if callable(path): - path_head, path_tail = os.path.split(path(self.meta.filename)) - else: - path_head, path_tail = os.path.split(path) - base, ext = os.path.splitext(path_tail) - if isinstance(ext, bytes): - ext = ext.decode(sys.getfilesystemencoding()) - - if dir_path: - path_head = dir_path - output_path = os.path.join(path_head, path_tail) + path = Path(path(self.meta.filename) if callable(path) else path) + output_path = Path(dir_path) / path.name if dir_path else path + ext = path.suffix.decode(sys.getfilesystemencoding()) if isinstance(path.suffix, bytes) else path.suffix # TODO: Support gzip-compressed fits if ext == ".asdf": From 5290be993b3f098de7b3c7084e35520d18ec8036 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Fri, 24 Nov 2023 12:56:37 -0500 Subject: [PATCH 2/5] Add test to demonstrate the problem --- tests/test_models.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_models.py b/tests/test_models.py index 766b0231..b27661d7 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -804,3 +804,15 @@ def test_datamodel_construct_like_from_like(model): new_mdl = model(mdl) assert new_mdl is mdl assert new_mdl._iscopy == "foo" # Verify that the constructor didn't override stuff + + +def test_datamodel_save_filename(tmp_path): + filename = tmp_path / "fancy_filename.asdf" + ramp = utils.mk_datamodel(datamodels.RampModel, shape=(2, 8, 8)) + assert ramp.meta.filename != filename.name + + ramp.save(filename) + assert ramp.meta.filename != filename.name + + with datamodels.open(filename) as new_ramp: + assert new_ramp.meta.filename == filename.name From 9c5fc90a6e0583fcc984bfba372da3f6b1e6e8f7 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Fri, 24 Nov 2023 12:40:50 -0500 Subject: [PATCH 3/5] Add adjustment to filename if needed. --- src/roman_datamodels/datamodels/_core.py | 25 ++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/roman_datamodels/datamodels/_core.py b/src/roman_datamodels/datamodels/_core.py index 031fd476..5241669e 100644 --- a/src/roman_datamodels/datamodels/_core.py +++ b/src/roman_datamodels/datamodels/_core.py @@ -12,6 +12,7 @@ import datetime import functools import sys +from contextlib import contextmanager from pathlib import Path, PurePath import asdf @@ -46,6 +47,22 @@ def wrapper(self, *args, **kwargs): return wrapper +@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 + + class DataModel(abc.ABC): """Base class for all top level datamodels""" @@ -196,10 +213,10 @@ def open_asdf(self, init=None, **kwargs): return asdf.open(init, **kwargs) if isinstance(init, str) else asdf.AsdfFile(init, **kwargs) def to_asdf(self, init, *args, **kwargs): - with validate.nuke_validation(): - asdffile = self.open_asdf(**kwargs) - asdffile.tree = {"roman": self._instance} - asdffile.write_to(init, *args, **kwargs) + with validate.nuke_validation(), _temporary_update_filename(self, Path(init).name): + asdf_file = self.open_asdf(**kwargs) + asdf_file.tree = {"roman": self._instance} + asdf_file.write_to(init, *args, **kwargs) def get_primary_array_name(self): """ From 12397d8e65da63a3b1b2d82c5b0fec6a97590b08 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Fri, 24 Nov 2023 13:46:56 -0500 Subject: [PATCH 4/5] Update changes --- CHANGES.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 88d87dfa..3d2b36c2 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,6 +4,8 @@ - Allow assignment to or creation of node attributes using dot notation of object instances with validation. [#284] +- Bugfix for ``model.meta.filename`` not matching the filename of the file on disk. [#295] + 0.18.0 (2023-11-06) =================== From 762dc7a7caff6dd31115c3ffd8aeaf2890b5024d Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Mon, 27 Nov 2023 11:15:40 -0500 Subject: [PATCH 5/5] Fix broken behavior --- src/roman_datamodels/datamodels/_core.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/roman_datamodels/datamodels/_core.py b/src/roman_datamodels/datamodels/_core.py index 5241669e..0cd27761 100644 --- a/src/roman_datamodels/datamodels/_core.py +++ b/src/roman_datamodels/datamodels/_core.py @@ -61,6 +61,10 @@ def _temporary_update_filename(datamodel, filename): yield datamodel._instance.meta.filename = old_filename + return + + yield + return class DataModel(abc.ABC):