Merge branch 'main' of https://github.com/spacetelescope/roman_datamodels into RCAL-701_L3Metadata

Author: PaulHuwe

.pre-commit-config.yaml

@@ -42,23 +42,23 @@ repos:
       args: ["--py38-plus"]
 
 - repo: https://github.com/astral-sh/ruff-pre-commit
-  rev: 'v0.1.3'
+  rev: 'v0.1.8'
   hooks:
     - id: ruff
       args: ["--fix"]
 
 - repo: https://github.com/pycqa/isort
-  rev: 5.12.0
+  rev: 5.13.2
   hooks:
     - id: isort
 
 - repo: https://github.com/psf/black
-  rev: 23.10.1
+  rev: 23.12.0
   hooks:
     - id: black
 
 - repo: https://github.com/PyCQA/bandit
-  rev: 1.7.5
+  rev: 1.7.6
   hooks:
     - id: bandit
       args: ["-r", "-ll"]

CHANGES.rst

@@ -1,7 +1,12 @@
 0.18.1 (unreleased)
 ===================
 
--
+- 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]
+
+- Bugfix for ``meta.model_type`` not being set to match the model writing the file. [#296]
 
 0.18.0 (2023-11-06)
 ===================

src/roman_datamodels/datamodels/_core.py

@@ -11,10 +11,9 @@
 import copy
 import datetime
 import functools
-import os
-import os.path
 import sys
-from pathlib import PurePath
+from contextlib import contextmanager
+from pathlib import Path, PurePath
 
 import asdf
 import numpy as np
@@ -48,6 +47,26 @@ 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
+        return
+
+    yield
+    return
+
+
 class DataModel(abc.ABC):
     """Base class for all top level datamodels"""
 
@@ -181,17 +200,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":
@@ -206,10 +217,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):
         """

src/roman_datamodels/datamodels/_datamodels.py

@@ -25,13 +25,26 @@ class _DataModel(DataModel):
     def __init_subclass__(cls, **kwargs):
         """Register each subclass in the __all__ for this module"""
         super().__init_subclass__(**kwargs)
+
+        # Don't register private classes
+        if cls.__name__.startswith("_"):
+            return
+
         if cls.__name__ in __all__:
             raise ValueError(f"Duplicate model type {cls.__name__}")
 
         __all__.append(cls.__name__)
 
 
-class MosaicModel(_DataModel):
+class _RomanDataModel(_DataModel):
+    def __init__(self, init=None, **kwargs):
+        super().__init__(init, **kwargs)
+
+        if init is not None:
+            self.meta.model_type = self.__class__.__name__
+
+
+class MosaicModel(_RomanDataModel):
     _node_type = stnode.WfiMosaic
 
     def append_individual_image_meta(self, meta):
@@ -50,19 +63,19 @@ def append_individual_image_meta(self, meta):
             self.meta.individual_image_meta.all_meta.append(meta.to_flat_dict())
 
 
-class ImageModel(_DataModel):
+class ImageModel(_RomanDataModel):
     _node_type = stnode.WfiImage
 
 
-class ScienceRawModel(_DataModel):
+class ScienceRawModel(_RomanDataModel):
     _node_type = stnode.WfiScienceRaw
 
 
-class MsosStackModel(_DataModel):
+class MsosStackModel(_RomanDataModel):
     _node_type = stnode.MsosStack
 
 
-class RampModel(_DataModel):
+class RampModel(_RomanDataModel):
     _node_type = stnode.Ramp
 
     @classmethod
@@ -101,7 +114,7 @@ def from_science_raw(cls, model):
         raise ValueError("Input model must be a ScienceRawModel or RampModel")
 
 
-class RampFitOutputModel(_DataModel):
+class RampFitOutputModel(_RomanDataModel):
     _node_type = stnode.RampFitOutput
 
 
@@ -122,7 +135,7 @@ def is_association(cls, asn_data):
         return isinstance(asn_data, dict) and "asn_id" in asn_data and "asn_pool" in asn_data
 
 
-class GuidewindowModel(_DataModel):
+class GuidewindowModel(_RomanDataModel):
     _node_type = stnode.Guidewindow
 
 

src/roman_datamodels/stnode/_node.py

@@ -94,6 +94,33 @@ def _get_schema_for_property(schema, attr):
     return {}
 
 
+def _get_attributes_from_schema(schema):
+    explicit_properties = schema.get("properties", {}).keys()
+    patterns = schema.get("patternProperties", {})
+    return SchemaProperties(explicit_properties, patterns)
+
+
+class SchemaProperties:
+    """
+    This class provides the capability for using the "contains" machinery
+    so that an attribute can be tested against patternProperties as well
+    as whether the attribute is explicitly a property of the schema.
+    """
+
+    def __init__(self, explicit_properties, patterns):
+        self.explicit_properties = explicit_properties
+        self.patterns = patterns
+
+    def __contains__(self, attr):
+        if attr in self.explicit_properties:
+            return True
+        else:
+            for key in self.patterns.keys():
+                if re.match(key, attr):
+                    return True
+        return False
+
+
 class DNode(MutableMapping):
     """
     Base class describing all "object" (dict-like) data nodes for STNode classes.
@@ -113,6 +140,7 @@ def __init__(self, node=None, parent=None, name=None):
         self._schema_uri = None
         self._parent = parent
         self._name = name
+        self._x_schema_attributes = None
 
     @property
     def ctx(self):
@@ -153,7 +181,7 @@ def __setattr__(self, key, value):
 
         if key[0] != "_":
             value = self._convert_to_scalar(key, value)
-            if key in self._data:
+            if key in self._data or key in self._schema_attributes():
                 if will_validate():
                     schema = _get_schema_for_property(self._schema(), key)
                     if schema:
@@ -164,6 +192,17 @@ def __setattr__(self, key, value):
         else:
             self.__dict__[key] = value
 
+    def _schema_attributes(self):
+        if self._x_schema_attributes is None:
+            self._x_schema_attributes = self._get_schema_attributes()
+        return self._x_schema_attributes
+
+    def _get_schema_attributes(self):
+        """
+        Extract all schema attributes for this node.
+        """
+        return _get_attributes_from_schema(self._schema())
+
     def to_flat_dict(self, include_arrays=True):
         """
         Returns a dictionary of all of the schema items as a flat dictionary.

tests/test_maker_utils.py

@@ -7,6 +7,7 @@
 from astropy.time import Time
 
 from roman_datamodels import datamodels, maker_utils, stnode
+from roman_datamodels.datamodels._datamodels import _RomanDataModel
 from roman_datamodels.maker_utils import _ref_files as ref_files
 from roman_datamodels.testing import assert_node_equal
 
@@ -109,6 +110,9 @@ def test_datamodel_maker(model_class):
     assert isinstance(model, model_class)
     model.validate()
 
+    if issubclass(model_class, _RomanDataModel):
+        assert model.meta.model_type == model_class.__name__
+
 
 @pytest.mark.parametrize("node_class", [node for node in datamodels.MODEL_REGISTRY])
 @pytest.mark.filterwarnings("ignore:This function assumes shape is 2D")

tests/test_models.py

@@ -807,6 +807,14 @@ def test_ramp_from_science_raw():
         if isinstance(ramp_value, np.ndarray):
             assert_array_equal(ramp_value, raw_value.astype(ramp_value.dtype))
 
+        elif key == "meta":
+            for meta_key in ramp_value:
+                if meta_key == "model_type":
+                    ramp_value[meta_key] = ramp.__class__.__name__
+                    raw_value[meta_key] = raw.__class__.__name__
+                    continue
+                assert_node_equal(ramp_value[meta_key], raw_value[meta_key])
+
         elif isinstance(ramp_value, stnode.DNode):
             assert_node_equal(ramp_value, raw_value)
 
@@ -839,3 +847,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

tests/test_stnode.py

@@ -4,8 +4,12 @@
 import asdf
 import astropy.units as u
 import pytest
+from asdf.exceptions import ValidationError
 
-from roman_datamodels import datamodels, maker_utils, stnode, validate
+from roman_datamodels import datamodels
+from roman_datamodels import maker_utils
+from roman_datamodels import maker_utils as utils
+from roman_datamodels import stnode, validate
 from roman_datamodels.testing import assert_node_equal, assert_node_is_copy, wraps_hashable
 
 from .conftest import MANIFEST
@@ -181,6 +185,26 @@ def test_set_pattern_properties():
     mdl.phot_table.F062.pixelareasr = None
 
 
+# Test that a currently undefined attribute can be assigned using dot notation
+# so long as the attribute is defined in the corresponding schema.
+def test_node_new_attribute_assignment():
+    exp = stnode.Exposure()
+    with pytest.raises(AttributeError):
+        exp.bozo = 0
+    exp.ngroups = 5
+    assert exp.ngroups == 5
+    # Test patternProperties attribute case
+    photmod = utils.mk_wfi_img_photom()
+    phottab = photmod.phot_table
+    newphottab = {"F062": phottab["F062"]}
+    photmod.phot_table = newphottab
+    photmod.phot_table.F213 = phottab["F213"]
+    with pytest.raises(AttributeError):
+        photmod.phot_table.F214 = phottab["F213"]
+    with pytest.raises(ValidationError):
+        photmod.phot_table.F106 = 0
+
+
 VALIDATION_CASES = ("true", "yes", "1", "True", "Yes", "TrUe", "YeS", "foo", "Bar", "BaZ")