Repository: yt-project/unyt (python)

Issue:
Creating UnitSystem based on actual "units" doesn't appear to work in 2.0.0
* unyt version: 2.0.0
* Python version: 3.7.2
* Operating System: MacOS

### Description

I have a custom unit system, called `cosmological`, that I use for my thin i/o wrapper for SWIFT. It's defined in the following way:

```
import unyt

cosmo_units = unyt.UnitSystem(
    "cosmological", unyt.Mpc, 1e10 * unyt.msun, unyt.s * unyt.Mpc / unyt.km
)
```
This broke our test suite in `unyt2.0.0`.

### What I Did

Minimal breaking example below:
```
>>> import unyt
>>> unyt.UnitSystem("cosmological", "Mpc", "Msun", "s")
cosmological Unit System
 Base Units:
  length: Mpc
  mass: Msun
  time: s
  temperature: K
  angle: rad
  current_mks: A
  luminous_intensity: cd
 Other Units:
>>> unyt.UnitSystem("cosmologicalRealUnits", unyt.Mpc, unyt.Msun, unyt.s)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/mphf18/Documents/swift/swiftsimio/env/lib/python3.7/site-packages/unyt/unit_systems.py", line 218, in __init__
    bu = _split_prefix(unit, default_lut)[1]
  File "/Users/mphf18/Documents/swift/swiftsimio/env/lib/python3.7/site-packages/unyt/unit_systems.py", line 111, in _split_prefix
    possible_prefix = symbol_str[0]
TypeError: 'Unit' object does not support indexing
```
This seems to work fine if I pass strings, but if I pass real units then it doesn't work. This is a pain as I want to use 1e10 solar masses as my mass unit.

Am I doing something wrong here?


Pull request:
make sure unit systems can be defined using unit objects. fixes #71
ping @JBorrow

It looks like this may have used to work by accident. However it seems like a natural thing to want to do so I've explicitly added support for it, including docs and tests.

Patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index ee1e0c9..3706ca3 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -39,7 +39,7 @@ you run into issues please let us know by `opening an issue on GitHub
   .. code-block:: python
 
      >>> from unyt import meter, second
-     >>> data = 1000*meter/second
+     >>> data = 1000.*meter/second
      >>> data.to('kilometer/second')
      unyt_quantity(1., 'km/s')
      >>> data.to('metre/s')
@@ -57,8 +57,9 @@ you run into issues please let us know by `opening an issue on GitHub
   a warning message will be printed. See `PR #55
   <https://github.com/yt-project/unyt/pull/55>`_ for details. This change may
   cause data to be loaded into ``unyt`` with a different dtype. On Windows the
-  default float dtype is ``float32``, so data may begin to be recognized as
-  ``float32`` by default.
+  default integer dtype is ``int32``, so data may begin to be recognized as
+  ``int32`` or converted to ``float32`` where before it was interpreted as
+  ``float64`` by default.
 * Unit registries are now associated with a unit system. This means that it's
   possible to create a unit registry that is associated with a non-MKS unit
   system so that conversions to "base" units will end up in that non-MKS
diff --git a/docs/usage.rst b/docs/usage.rst
index c7d7f8f..70ffbf3 100644
--- a/docs/usage.rst
+++ b/docs/usage.rst
@@ -467,6 +467,22 @@ atoms and molecules:
    >>> atomic_unit_system['angular_momentum']
    mp*nm**2/fs
 
+It is also legal to define a unit system using :class:`unyt.Unit
+<unyt.unit_object.Unit>` instances:
+
+  >>> from unyt.unit_symbols import Msun, second, megaparsec
+  >>> UnitSystem('cosmological', megaparsec, Msun, second)
+    cosmological Unit System
+     Base Units:
+      length: Mpc
+      mass: Msun
+      time: s
+      temperature: K
+      angle: rad
+      current_mks: A
+      luminous_intensity: cd
+     Other Units:
+
 Once you have defined a new unit system that will register the new system with a
 global registry of unit systems known to the :mod:`unyt` library. That means you
 will immediately be able to use it just like the built-in unit systems:
diff --git a/unyt/unit_registry.py b/unyt/unit_registry.py
index 1fb8354..46ff81a 100644
--- a/unyt/unit_registry.py
+++ b/unyt/unit_registry.py
@@ -57,11 +57,11 @@ class UnitRegistry:
 
     def __getitem__(self, key):
         try:
-            ret = self.lut[key]
+            ret = self.lut[str(key)]
         except KeyError:
             try:
                 _lookup_unit_symbol(str(key), self.lut)
-                ret = self.lut[key]
+                ret = self.lut[str(key)]
             except UnitParseError:
                 raise SymbolNotFoundError(
                     "The symbol '%s' does not exist in this registry." % key
diff --git a/unyt/unit_systems.py b/unyt/unit_systems.py
index 9aa3d2c..ae770a0 100644
--- a/unyt/unit_systems.py
+++ b/unyt/unit_systems.py
@@ -161,25 +161,25 @@ class UnitSystem(object):
     name : string
         The name of the unit system. Will be used as the key in the
         *unit_system_registry* dict to reference the unit system by.
-    length_unit : string
+    length_unit : string or :class:`unyt.unit_object.Unit`
         The base length unit of this unit system.
-    mass_unit : string
+    mass_unit : string or :class:`unyt.unit_object.Unit`
         The base mass unit of this unit system.
-    time_unit : string
+    time_unit : string or :class:`unyt.unit_object.Unit`
         The base time unit of this unit system.
-    temperature_unit : string, optional
+    temperature_unit : string or :class:`unyt.unit_object.Unit`, optional
         The base temperature unit of this unit system. Defaults to "K".
-    angle_unit : string, optional
+    angle_unit : string or :class:`unyt.unit_object.Unit`, optional
         The base angle unit of this unit system. Defaults to "rad".
     mks_system: boolean, optional
         Whether or not this unit system has SI-specific units.
         Default: False
-    current_mks_unit : string, optional
+    current_mks_unit : string or :class:`unyt.unit_object.Unit`, optional
         The base current unit of this unit system. Defaults to "A".
-    luminous_intensity_unit : string, optional
+    luminous_intensity_unit : string or :class:`unyt.unit_object.Unit`, optional
         The base luminous intensity unit of this unit system.
         Defaults to "cd".
-    registry : :class:`yt.units.unit_registry.UnitRegistry` object
+    registry : :class:`unyt.unit_registry.UnitRegistry` object
         The unit registry associated with this unit system. Only
         useful for defining unit systems based on code units.
     """
@@ -215,8 +215,12 @@ class UnitSystem(object):
             if self.registry is not None and self.registry[unit][1] is not dimension:
                 raise IllDefinedUnitSystem(self.units_map)
             elif self.registry is None:
-                bu = _split_prefix(unit, default_lut)[1]
-                if default_lut[inv_name_alternatives[bu]][1] is not dimension:
+                if isinstance(unit, str):
+                    bu = _split_prefix(unit, default_lut)[1]
+                    inferred_dimension = default_lut[inv_name_alternatives[bu]][1]
+                else:
+                    inferred_dimension = getattr(unit, "dimensions", None)
+                if inferred_dimension is not dimension:
                     raise IllDefinedUnitSystem(self.units_map)
         self._dims = [
             "length",


Test patch:
diff --git a/unyt/tests/test_unit_systems.py b/unyt/tests/test_unit_systems.py
index 546a0a9..adf7541 100644
--- a/unyt/tests/test_unit_systems.py
+++ b/unyt/tests/test_unit_systems.py
@@ -13,6 +13,8 @@ Test unit systems.
 
 import pytest
 
+import unyt.unit_symbols as us
+
 from unyt.exceptions import IllDefinedUnitSystem, MissingMKSCurrent
 from unyt.unit_object import Unit
 from unyt.unit_systems import UnitSystem, cgs_unit_system, unit_system_registry
@@ -61,6 +63,10 @@ def test_bad_unit_system():
         UnitSystem("atomic", "nm", "fs", "nK", "rad")
     with pytest.raises(IllDefinedUnitSystem):
         UnitSystem("atomic", "nm", "fs", "nK", "rad", registry=UnitRegistry())
+    with pytest.raises(IllDefinedUnitSystem):
+        UnitSystem("atomic", us.nm, us.fs, us.nK, us.rad)
+    with pytest.raises(IllDefinedUnitSystem):
+        UnitSystem("atomic", us.nm, us.fs, us.nK, us.rad, registry=UnitRegistry())
 
 
 def test_mks_current():
@@ -72,3 +78,10 @@ def test_mks_current():
         cgs_unit_system[dimensions.current_mks] = "foo"
     with pytest.raises(MissingMKSCurrent):
         cgs_unit_system[dimensions.magnetic_field] = "bar"
+
+
+def test_create_unit_system_from_unit_objects():
+    s = UnitSystem("test_units", us.Mpc, us.Msun, us.s)
+    assert s["length"] == us.Mpc
+    assert s["mass"] == us.Msun
+    assert s["time"] == us.s