Remove the target keyword from rdm_open

CHANGES.rst

@@ -9,6 +9,10 @@
 
 - Clean up overlooked randomness in ``maker_utils`` and tests. [#236]
 
+- Remove the unused ``target`` keyword from ``rdm_open`` and fix the original issue that the
+  keyword was meant to address; namely, passing a datamodel instance to the constructor for
+  that datamodel instance should return the instance back with no modifications. [#235]
+
 0.16.1 (2023-06-27)
 ===================
 

src/roman_datamodels/datamodels/_core.py

@@ -56,7 +56,22 @@ def __init_subclass__(cls, **kwargs):
         # Add to registry
         MODEL_REGISTRY[cls._node_type] = cls
 
+    def __new__(cls, init=None, **kwargs):
+        """
+        Handle the case where one passes in an already instantiated version
+        of the model. In this case the constructor should just directly return
+        the model.
+        """
+        if init.__class__.__name__ == cls.__name__:
+            return init
+
+        return super().__new__(cls)
+
     def __init__(self, init=None, **kwargs):
+        if isinstance(init, self.__class__):
+            # Due to __new__ above, this is already initialized.
+            return
+
         self._iscopy = False
         self._shape = None
         self._instance = None

src/roman_datamodels/datamodels/_utils.py

@@ -28,7 +28,7 @@
 __all__ = ["rdm_open"]
 
 
-def rdm_open(init, memmap=False, target=None, **kwargs):
+def rdm_open(init, memmap=False, **kwargs):
     """
     Datamodel open/create function.
         This function opens a Roman datamodel from an asdf file or generates
@@ -43,34 +43,19 @@ def rdm_open(init, memmap=False, target=None, **kwargs):
             - `DataModel` Roman data model instance
     memmap : bool
         Open ASDF file binary data using memmap (default: False)
-    target : `DataModel`
-        If not None value, the `DataModel` implied by the init argument
-        must be an instance of the target class. If the init value
-        is already a data model, and matches the target, the init
-        value is returned, not copied, as opposed to the case where
-        the init value is a data model, and target is not supplied,
-        and the returned value is a copy of the init value.
 
     Returns
     -------
     `DataModel`
     """
     with validate.nuke_validation():
         file_to_close = None
-        if target is not None:
-            if not issubclass(target, DataModel):
-                raise ValueError("Target must be a subclass of DataModel")
         # Temp fix to catch JWST args before being passed to asdf open
         if "asn_n_members" in kwargs:
             del kwargs["asn_n_members"]
         if isinstance(init, asdf.AsdfFile):
             asdffile = init
         elif isinstance(init, DataModel):
-            if target is not None:
-                if not isinstance(init, target):
-                    raise ValueError("First argument is not an instance of target")
-                else:
-                    return init
             # Copy the object so it knows not to close here
             return init.copy()
         else:
@@ -86,13 +71,6 @@ def rdm_open(init, memmap=False, target=None, **kwargs):
                 raise TypeError("Roman datamodels does not accept FITS files or objects")
         modeltype = type(asdffile.tree["roman"])
         if modeltype in MODEL_REGISTRY:
-            rmodel = MODEL_REGISTRY[modeltype](asdffile, **kwargs)
-            if target is not None:
-                if not issubclass(rmodel.__class__, target):
-                    if file_to_close is not None:
-                        file_to_close.close()
-                    raise ValueError("Referenced ASDF file model type is not subclass of target")
-            else:
-                return rmodel
+            return MODEL_REGISTRY[modeltype](asdffile, **kwargs)
         else:
             return DataModel(asdffile, **kwargs)

tests/test_models.py

@@ -691,3 +691,30 @@ def test_ramp_from_science_raw():
 
         else:
             raise ValueError(f"Unexpected type {type(ramp_value)}, {key}")
+
+
+@pytest.mark.parametrize("model", datamodels.MODEL_REGISTRY.values())
+@pytest.mark.filterwarnings("ignore:This function assumes shape is 2D")
+@pytest.mark.filterwarnings("ignore:Input shape must be 5D")
+def test_datamodel_construct_like_from_like(model):
+    """
+    This is a regression test for the issue reported issue #51.
+
+    Namely, if one passes a datamodel instance to the constructor for the datamodel
+    of the same type as the instance, an error should not be raised (#51 reports an
+    error being raised).
+
+    Based on the discussion in PR #52, this should return exactly the same instance object
+    that was passed to the constructor. i.e. it should not return a copy of the instance.
+    """
+
+    # Create a model
+    mdl = utils.mk_datamodel(model, shape=(2, 8, 8))
+
+    # Modify _iscopy as it will be reset to False by initializer if called incorrectly
+    mdl._iscopy = "foo"
+
+    # Pass model instance to model constructor
+    new_mdl = model(mdl)
+    assert new_mdl is mdl
+    assert new_mdl._iscopy == "foo"  # Verify that the constructor didn't override stuff