Repository: yt-project/unyt (python)

Issue:
Attempting to define capacitance unit Farad raises UnitsNotReducible.
* unyt version: 1.0.7
* Python version: 3.6.6
* Operating System: Win10

### Description

I tried to define the capacitance unit Farad.

### What I Did

```
import unyt as u

u.define_unit('F', (1.0, 'C/V'), r'\rm{F}', None, True)
---------------------------------------------------------------------------
MKSCGSConversionError                     Traceback (most recent call last)
c:\Python\projects\unyt\unyt\array.py in in_base(self, unit_system)
    897             conv_data = _check_em_conversion(
--> 898                 self.units, unit_system=us, registry=self.units.registry)
    899         except MKSCGSConversionError:

c:\Python\projects\unyt\unyt\unit_object.py in _check_em_conversion(unit, to_unit, unit_system, registry)
    810             else:
--> 811                 raise MKSCGSConversionError(unit)
    812     return em_map

MKSCGSConversionError: The 'C/V' unit cannot be safely converted.

During handling of the above exception, another exception occurred:

UnitsNotReducible                         Traceback (most recent call last)
<ipython-input-2-452d0c29d592> in <module>
----> 1 u.define_unit('F', (1.0, 'C/V'), r'\rm{F}', None, True)

c:\Python\projects\unyt\unyt\unit_object.py in define_unit(symbol, value, tex_repr, offset, prefixable, registry)
    987             raise RuntimeError("\"value\" needs to be a quantity or "
    988                                "(value, unit) tuple!")
--> 989     base_value = float(value.in_base(unit_system='mks'))
    990     dimensions = value.units.dimensions
    991     registry.add(symbol, base_value, dimensions, prefixable=prefixable,

c:\Python\projects\unyt\unyt\array.py in in_base(self, unit_system)
    898                 self.units, unit_system=us, registry=self.units.registry)
    899         except MKSCGSConversionError:
--> 900             raise UnitsNotReducible(self.units, unit_system)
    901         if any(conv_data):
    902             to_units, (conv, offset) = _em_conversion(

UnitsNotReducible: The unit "C/V" (dimensions "(current_mks)**2*(time)**4/((length)**2*(mass))") cannot be reduced to an expression within the mks system of units.
```


Pull request:
make MKS <-> CGS conversions smarter, add Farad. Fixes #53
This fixes the issue pointed out by @l-johnston in #53. For units that are made up of base units we can be a little bit smarter. I've also added a test.

I've also added Farad to the default unit registry since I believe that was omitted as an oversight and not because of any intentional reason.

Edit: Also pushed some minor doc fixes to unbreak the coverage tests.

Patch:
diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
index 8ca5ace..6273ecc 100644
--- a/CONTRIBUTING.rst
+++ b/CONTRIBUTING.rst
@@ -95,7 +95,7 @@ Ready to contribute? Here's how to set up ``unyt`` for local development.
    tests, including testing other Python versions with tox::
 
     $ flake8 unyt
-    $ pytest --doctest-modules --doctest-glob='*.rst' --doctest-plus
+    $ pytest --doctest-modules --doctest-rst --doctest-plus
     $ tox
 
    To get flake8, pytest, pytest-doctestplus, and tox, just pip install them
@@ -116,7 +116,7 @@ We use the ``pytest`` test runner as well as the ``tox`` test wrapper to manage
 the ``unyt`` repository, simply run ``pytest`` in the root of the repository::
 
    $ cd unyt/
-   $ py.test --doctest-modules --doctest-glob='*.rst' --doctest-plus
+   $ py.test --doctest-modules --doctest-rst --doctest-plus
 
 You will need to install ``pytest`` and ``pytest-doctestplus`` from ``pip`` to run this command. Some tests depend on ``h5py``, ``Pint``, ``astropy``, and ``flake8`` being installed.
 
diff --git a/tox.ini b/tox.ini
index 08b1104..d9b41c0 100644
--- a/tox.ini
+++ b/tox.ini
@@ -27,7 +27,7 @@ deps =
     setuptools
     py27: backports.functools_lru_cache
 commands =
-    pytest --cov=unyt --cov-append --doctest-modules --doctest-plus --doctest-glob='*.rst' --basetemp={envtmpdir}
+    pytest --cov=unyt --cov-append --doctest-modules --doctest-plus --doctest-rst --basetemp={envtmpdir}
     coverage report --omit='.tox/*'
 
 [testenv:py27]
diff --git a/unyt/_unit_lookup_table.py b/unyt/_unit_lookup_table.py
index cd00b1c..7b3a460 100644
--- a/unyt/_unit_lookup_table.py
+++ b/unyt/_unit_lookup_table.py
@@ -90,9 +90,10 @@ default_unit_symbol_lut = {  # noqa: W605
     "C": (1.0, dimensions.charge_mks, 0.0, r"\rm{C}", True),
     "T": (1.0, dimensions.magnetic_field_mks, 0.0, r"\rm{T}", True),
     "Pa": (1.0, dimensions.pressure, 0.0, r"\rm{Pa}", True),
-    "V": (1.0, dimensions.electric_potential_mks, 0.0, r"\rm{V}", True),
-    "ohm": (1.0, dimensions.resistance_mks, 0.0, r"\Omega", True),
-    "Wb": (1.0, dimensions.magnetic_flux_mks, 0.0, r"\rm{Wb}", True),
+    "V": (1.0, dimensions.electric_potential, 0.0, r"\rm{V}", True),
+    "F": (1.0, dimensions.capacitance, 0.0, r"\rm{F}", True),
+    "ohm": (1.0, dimensions.resistance, 0.0, r"\Omega", True),
+    "Wb": (1.0, dimensions.magnetic_flux, 0.0, r"\rm{Wb}", True),
     "lm": (1.0, dimensions.luminous_flux, 0.0, r"\rm{lm}", True),
     "lx": (1.0, dimensions.luminous_flux/dimensions.area, 0.0, r"\rm{lx}",
            True),
diff --git a/unyt/dimensions.py b/unyt/dimensions.py
index 24afccd..0b19c39 100644
--- a/unyt/dimensions.py
+++ b/unyt/dimensions.py
@@ -117,17 +117,19 @@ resistance_cgs = electric_potential_cgs / current_cgs
 magnetic_flux_cgs = magnetic_field_cgs * area
 
 # SI electromagnetic units
-#: charge_mks
+#: charge
 charge = charge_mks = current_mks * time
-#: electric_field_mks
+#: electric_field
 electric_field = electric_field_mks = force / charge_mks
-#: magnetic_field_mks
+#: magnetic_field
 magnetic_field = magnetic_field_mks = electric_field_mks / velocity
-#: electric_potential_mks
+#: electric_potential
 electric_potential = electric_potential_mks = energy / charge_mks
-#: resistance_mks
+#: resistance
 resistance = resistance_mks = electric_potential_mks / current_mks
-#: magnetic_flux_mks
+#: capacitance
+capacitance = capacitance_mks = charge / electric_potential
+#: magnetic_flux
 magnetic_flux = magnetic_flux_mks = magnetic_field_mks * area
 
 #: a list containing all derived_dimensions
diff --git a/unyt/exceptions.py b/unyt/exceptions.py
index c40275a..a3fd778 100644
--- a/unyt/exceptions.py
+++ b/unyt/exceptions.py
@@ -21,7 +21,8 @@ class UnitOperationError(ValueError):
     -------
 
     >>> import unyt as u
-    >>> 3*u.g + 4*u.m  # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
+    >>> 3*u.g + 4*u.m\
+ # doctest: +IGNORE_EXCEPTION_DETAIL +NORMALIZE_WHITESPACE
     Traceback (most recent call last):
     ...
     unyt.exceptions.UnitOperationError: The <ufunc 'add'> operator
@@ -53,7 +54,7 @@ class UnitConversionError(Exception):
 
     >>> import unyt as u
     >>> data = 3*u.g
-    >>> data.to('m')  # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
+    >>> data.to('m')  # doctest: +IGNORE_EXCEPTION_DETAIL +NORMALIZE_WHITESPACE
     Traceback (most recent call last):
     ...
     unyt.exceptions.UnitConversionError: Cannot convert between 'g'
@@ -87,7 +88,8 @@ class MissingMKSCurrent(Exception):
 
     >>> from unyt.unit_systems import cgs_unit_system as us
     >>> from unyt import ampere
-    >>> us[ampere.dimensions]  # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
+    >>> us[ampere.dimensions]\
+ # doctest: +IGNORE_EXCEPTION_DETAIL +NORMALIZE_WHITESPACE
     Traceback (most recent call last):
     ...
     unyt.exceptions.MissingMKSCurrent: The cgs unit system does not
@@ -126,7 +128,8 @@ class UnitsNotReducible(Exception):
 
     >>> from unyt import A, cm
     >>> data = 12*A/cm
-    >>> data.in_cgs()  # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
+    >>> data.in_cgs()\
+  # doctest: +IGNORE_EXCEPTION_DETAIL +NORMALIZE_WHITESPACE
     Traceback (most recent call last):
     ...
     unyt.exceptions.UnitsNotReducible: The unit "A/cm" (dimensions
@@ -153,7 +156,8 @@ class IterableUnitCoercionError(Exception):
 
     >>> from unyt import km, cm, unyt_array
     >>> data = [2*cm, 3*km]
-    >>> unyt_array(data)  # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
+    >>> unyt_array(data)\
+  # doctest: +IGNORE_EXCEPTION_DETAIL +NORMALIZE_WHITESPACE
     Traceback (most recent call last):
     ...
     unyt.exceptions.IterableUnitCoercionError: Received a list or
@@ -178,7 +182,7 @@ class InvalidUnitEquivalence(Exception):
     >>> import unyt as u
     >>> data = 12*u.g
     >>> data.to('erg', equivalence='thermal')\
- # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
+ # doctest: +IGNORE_EXCEPTION_DETAIL +NORMALIZE_WHITESPACE
     Traceback (most recent call last):
     ...
     unyt.exceptions.InvalidUnitEquivalence: The unit equivalence
@@ -207,7 +211,7 @@ class InvalidUnitOperation(Exception):
     -------
 
     >>> from unyt import cm, g
-    >>> cm + g  # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
+    >>> cm + g  # doctest: +IGNORE_EXCEPTION_DETAIL +NORMALIZE_WHITESPACE
     Traceback (most recent call last):
     ...
     unyt.exceptions.InvalidUnitOperation: addition with unit objects
@@ -224,7 +228,7 @@ class SymbolNotFoundError(Exception):
 
     >>> from unyt.unit_registry import default_unit_registry
     >>> default_unit_registry['made_up_unit']\
-  # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
+  # doctest: +IGNORE_EXCEPTION_DETAIL +NORMALIZE_WHITESPACE
     Traceback (most recent call last):
     ...
     unyt.exceptions.SymbolNotFoundError: The symbol 'made_up_unit'
@@ -240,7 +244,8 @@ class UnitParseError(Exception):
     -------
 
     >>> from unyt import Unit
-    >>> Unit('hello')  # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
+    >>> Unit('hello')\
+  # doctest: +IGNORE_EXCEPTION_DETAIL +NORMALIZE_WHITESPACE
     Traceback (most recent call last):
     ...
     unyt.exceptions.UnitParseError: Could not find unit symbol
@@ -258,7 +263,7 @@ class IllDefinedUnitSystem(Exception):
 
     >>> from unyt.unit_systems import UnitSystem
     >>> UnitSystem('atomic', 'nm', 'fs', 'nK', 'rad')\
-  # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
+  # doctest: +IGNORE_EXCEPTION_DETAIL +NORMALIZE_WHITESPACE
     Traceback (most recent call last):
     ...
     unyt.exceptions.IllDefinedUnitSystem: Cannot create unit system
diff --git a/unyt/unit_object.py b/unyt/unit_object.py
index 167451f..24d7ec8 100644
--- a/unyt/unit_object.py
+++ b/unyt/unit_object.py
@@ -59,6 +59,7 @@ import unyt.dimensions as dims
 from unyt.equivalencies import equivalence_registry
 from unyt.exceptions import (
     InvalidUnitOperation,
+    MissingMKSCurrent,
     MKSCGSConversionError,
     UnitConversionError,
     UnitsNotReducible,
@@ -769,7 +770,7 @@ def _check_em_conversion(unit, to_unit=None, unit_system=None,
 
     This function supports unyt's ability to convert data to and from E&M
     electromagnetic units. However, this support is limited and only very
-    simple unit expressions can be readily converted. This function
+    simple unit expressions can be readily converted. This function tries
     to see if the unit is an atomic base unit that is present in the
     em_conversions dict. If it does not contain E&M units, the function
     returns an empty tuple. If it does contain an atomic E&M unit in
@@ -795,11 +796,22 @@ def _check_em_conversion(unit, to_unit=None, unit_system=None,
             em_map = (to_unit, em_unit, em_info[2])
     if em_map:
         return em_map
+    if unit_system is None:
+        if to_unit.dimensions != unit.dimensions:
+            raise MKSCGSConversionError(unit)
+        else:
+            from unyt.unit_systems import unit_system_registry
+            unit_system = unit_system_registry['mks']
     for unit_atom in unit.expr.atoms():
         if unit_atom.is_Number:
-            pass
+            continue
         bu = str(unit_atom)
         budims = Unit(bu, registry=registry).dimensions
+        try:
+            if str(unit_system[budims]) == bu:
+                continue
+        except MissingMKSCurrent:
+            raise MKSCGSConversionError(unit)
         if budims in em_conversions:
             conv_unit = em_conversions[budims][1]
             if to_unit is not None:
@@ -969,9 +981,9 @@ def define_unit(symbol, value, tex_repr=None, offset=None, prefixable=False,
     >>> from unyt import day
     >>> two_weeks = 14.0*day
     >>> one_day = 1.0*day
-    >>> define_unit("fortnight", two_weeks)
-    >>> from unyt import fortnight
-    >>> print((3*fortnight)/one_day)
+    >>> define_unit("two_weeks", two_weeks)
+    >>> from unyt import two_weeks
+    >>> print((3*two_weeks)/one_day)
     42.0 dimensionless
     """
     from unyt.array import unyt_quantity, _iterable
@@ -979,7 +991,9 @@ def define_unit(symbol, value, tex_repr=None, offset=None, prefixable=False,
     if registry is None:
         registry = default_unit_registry
     if symbol in registry:
-        registry.pop(symbol)
+        raise RuntimeError(
+            "Unit symbol '%s' already exists in the provided "
+            "registry" % symbol)
     if not isinstance(value, unyt_quantity):
         if _iterable(value) and len(value) == 2:
             value = unyt_quantity(value[0], value[1])
diff --git a/unyt/unit_symbols.py b/unyt/unit_symbols.py
index c0452a5..44fa599 100644
--- a/unyt/unit_symbols.py
+++ b/unyt/unit_symbols.py
@@ -336,6 +336,8 @@ T = tesla = Tesla = Unit("T")
 A = ampere = Ampere = Unit("A")
 #: Volt
 V = volt = Volt = Unit("V")
+#: Farad
+F = farad = Farad = Unit("F")
 #: Ohm
 ohm = Ohm = Unit("ohm")
 #: Weber
diff --git a/unyt/unit_systems.py b/unyt/unit_systems.py
index a0f4a53..03fdac0 100644
--- a/unyt/unit_systems.py
+++ b/unyt/unit_systems.py
@@ -145,9 +145,15 @@ mks_unit_system["energy"] = "J"
 mks_unit_system["specific_energy"] = "J/kg"
 mks_unit_system["pressure"] = "Pa"
 mks_unit_system["force"] = "N"
-mks_unit_system["magnetic_field_mks"] = "T"
-mks_unit_system["charge_mks"] = "C"
+mks_unit_system["magnetic_field"] = "T"
+mks_unit_system["charge"] = "C"
+mks_unit_system["frequency"] = "Hz"
 mks_unit_system["power"] = "W"
+mks_unit_system["electric_potential"] = "V"
+mks_unit_system["capacitance"] = "F"
+mks_unit_system["resistance"] = "ohm"
+mks_unit_system["magnetic_flux"] = "Wb"
+mks_unit_system["luminous_flux"] = "lx"
 
 #: The imperial unit system
 imperial_unit_system = UnitSystem("imperial", "ft", "lb", "s",


Test patch:
diff --git a/unyt/tests/test_define_unit.py b/unyt/tests/test_define_unit.py
index 2692ede..0f9639b 100644
--- a/unyt/tests/test_define_unit.py
+++ b/unyt/tests/test_define_unit.py
@@ -1,3 +1,5 @@
+import pytest
+
 from unyt.unit_object import define_unit
 from unyt.array import unyt_quantity
 
@@ -9,7 +11,24 @@ def test_define_unit():
     c = unyt_quantity(1.0, "hr")
     assert a == 2.0*b/c
     d = unyt_quantity(1000.0, "cm**3")
-    define_unit("L", d, prefixable=True)
-    e = unyt_quantity(1.0, "mL")
+    define_unit("Baz", d, prefixable=True)
+    e = unyt_quantity(1.0, "mBaz")
     f = unyt_quantity(1.0, "cm**3")
     assert e == f
+
+    define_unit("Foo", (1.0, "V/sqrt(s)"))
+    g = unyt_quantity(1.0, 'Foo')
+    volt = unyt_quantity(1.0, 'V')
+    second = unyt_quantity(1.0, 's')
+    assert g == volt/second**(0.5)
+
+
+def test_define_unit_error():
+    from unyt import define_unit
+
+    with pytest.raises(RuntimeError):
+        define_unit('foobar', 'baz')
+    with pytest.raises(RuntimeError):
+        define_unit('foobar', 12)
+    with pytest.raises(RuntimeError):
+        define_unit('C', (1.0, 'A*s'))
diff --git a/unyt/tests/test_units.py b/unyt/tests/test_units.py
index a41fac5..c54feb8 100644
--- a/unyt/tests/test_units.py
+++ b/unyt/tests/test_units.py
@@ -654,15 +654,6 @@ def test_em_unit_base_equivalent():
         (A/cm).get_base_equivalent('cgs')
 
 
-def test_define_unit_error():
-    from unyt import define_unit
-
-    with pytest.raises(RuntimeError):
-        define_unit('foobar', 'baz')
-    with pytest.raises(RuntimeError):
-        define_unit('foobar', 12)
-
-
 def test_symbol_lut_length():
     for v in default_unit_symbol_lut.values():
         assert len(v) == 5
diff --git a/unyt/tests/test_unyt_array.py b/unyt/tests/test_unyt_array.py
index ddab37e..891ffa0 100644
--- a/unyt/tests/test_unyt_array.py
+++ b/unyt/tests/test_unyt_array.py
@@ -1674,8 +1674,7 @@ def test_electromagnetic():
     u_cgs = B_cgs*B_cgs/(8*np.pi)
     with pytest.raises(UnitConversionError):
         u_cgs.to(u_mks.units)
-    with pytest.raises(UnitConversionError):
-        u_mks.to(u_cgs.units)
+    assert_equal(u_mks.to(u_cgs.units), u_cgs)
     with pytest.raises(UnitsNotReducible):
         u_mks.in_cgs()
     with pytest.raises(UnitsNotReducible):