Repository: yt-project/unyt
Pull request #140 (2020-02-13 05:58:33)

Fix bugs spotted by the yt tests
The addition of `name` to the `__new__` signature broke existing subclasses(like yt's `ImageArray`) that don't have `name` in their `__new__` signature. As a bandaid I've wrapped the places `__new__` gets called in try/except blocks.

I also fixed a logic error that caused custom units to not be loaded correctly from jsons or pickles.

I've added tests that would catch both of these issues so hopefully we won't have future recurrances of these regressions.

It may make sense to run a test job that runs the yt unit tests since it seems to be able to find more bugs thant he unyt tests alone.

ping @l-johnston for the changes to how `name` works - maybe there's a better way than try/except?


Patch:
diff --git a/unyt/array.py b/unyt/array.py
index 1992141..d685f29 100644
--- a/unyt/array.py
+++ b/unyt/array.py
@@ -824,9 +824,13 @@ class unyt_array(np.ndarray):
             if offset:
                 np.subtract(ret, offset, ret)
 
-            new_array = type(self)(
-                ret, new_units, bypass_validation=True, name=self.name
-            )
+            try:
+                new_array = type(self)(
+                    ret, new_units, bypass_validation=True, name=self.name
+                )
+            except TypeError:
+                # subclasses might not take name as a kwarg
+                new_array = type(self)(ret, new_units, bypass_validation=True)
 
             return new_array
         else:
@@ -1860,7 +1864,11 @@ class unyt_array(np.ndarray):
 
         """
         name = getattr(self, "name", None)
-        return type(self)(np.copy(np.asarray(self)), self.units, name=name)
+        try:
+            return type(self)(np.copy(np.asarray(self)), self.units, name=name)
+        except TypeError:
+            # subclasses might not take name as a kwarg
+            return type(self)(np.copy(np.asarray(self)), self.units)
 
     def __array_finalize__(self, obj):
         self.units = getattr(obj, "units", NULL_UNIT)
@@ -1936,7 +1944,11 @@ class unyt_array(np.ndarray):
         This is necessary for stdlib deepcopy of arrays and quantities.
         """
         ret = super(unyt_array, self).__deepcopy__(memodict)
-        return type(self)(ret, copy.deepcopy(self.units), name=self.name)
+        try:
+            return type(self)(ret, copy.deepcopy(self.units), name=self.name)
+        except TypeError:
+            # subclasses might not take name as a kwarg
+            return type(self)(ret, copy.deepcopy(self.units))
 
 
 class unyt_quantity(unyt_array):
diff --git a/unyt/unit_registry.py b/unyt/unit_registry.py
index d83f1bc..c0cdbf8 100644
--- a/unyt/unit_registry.py
+++ b/unyt/unit_registry.py
@@ -352,7 +352,7 @@ def _correct_old_unit_registry(data, sympify=False):
                     unsan_v[0] /= 1000 ** float(power)
                 if dim == unyt_dims.length:
                     unsan_v[0] /= 100 ** float(power)
-            lut[k] = tuple(unsan_v)
+        lut[k] = tuple(unsan_v)
     for k in default_unit_symbol_lut:
         if k not in lut:
             lut[k] = default_unit_symbol_lut[k]


Test patch:
diff --git a/unyt/tests/test_unit_registry.py b/unyt/tests/test_unit_registry.py
index 74fcc87..917dd0f 100644
--- a/unyt/tests/test_unit_registry.py
+++ b/unyt/tests/test_unit_registry.py
@@ -81,6 +81,7 @@ def test_registry_contains():
 
 def test_registry_json():
     reg = UnitRegistry()
+    reg.add("tayne", 1.0, length)
     json_reg = reg.to_json()
     unserialized_reg = UnitRegistry.from_json(json_reg)
 
@@ -88,6 +89,7 @@ def test_registry_json():
 
     assert reg.lut["m"][1] is length
     assert reg.lut["erg"][1] is energy
+    assert reg.lut["tayne"][1] is length
 
 
 def test_old_registry_json():
diff --git a/unyt/tests/test_unyt_array.py b/unyt/tests/test_unyt_array.py
index e8c9493..98cbaf8 100644
--- a/unyt/tests/test_unyt_array.py
+++ b/unyt/tests/test_unyt_array.py
@@ -1395,7 +1395,16 @@ def test_pint():
 
 def test_subclass():
     class unyt_a_subclass(unyt_array):
-        pass
+        def __new__(
+            cls, input_array, units=None, registry=None, bypass_validation=None
+        ):
+            return super(unyt_a_subclass, cls).__new__(
+                cls,
+                input_array,
+                units,
+                registry=registry,
+                bypass_validation=bypass_validation,
+            )
 
     a = unyt_a_subclass([4, 5, 6], "g")
     b = unyt_a_subclass([7, 8, 9], "kg")
@@ -1428,6 +1437,9 @@ def test_subclass():
     assert_isinstance(a[:], unyt_a_subclass)
     assert_isinstance(a[:2], unyt_a_subclass)
     assert_isinstance(unyt_a_subclass(yta), unyt_a_subclass)
+    assert_isinstance(a.to("kg"), unyt_a_subclass)
+    assert_isinstance(a.copy(), unyt_a_subclass)
+    assert_isinstance(copy.deepcopy(a), unyt_a_subclass)
 
     with pytest.raises(RuntimeError):
         a + "hello"