Address a bunch of review comments

Cantera · Apr 25, 2023 · ed4b3a9 · ed4b3a9
1 parent 1672898
commit ed4b3a9
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 92 deletions.
diff --git a/doc/sphinx/conf.py b/doc/sphinx/conf.py
@@ -84,48 +84,18 @@ def escape_splats(app, what, name, obj, options, lines):
                     lines[i] = l.replace("*", r"\*")
     app.connect('autodoc-process-docstring', escape_splats)
 
-    # NAMES = []
-    # DIRECTIVES = {}
-
-    # def get_rst(app, what, name, obj, options, signature,
-    #             return_annotation):
-    #     if "with_units" not in name:
-    #         return
-    #     doc_indent = '    '
-    #     directive_indent = ''
-    #     if what in ['method', 'attribute']:
-    #         doc_indent += '    '
-    #         directive_indent += '    '
-    #     directive = '%s.. py:%s:: %s' % (directive_indent, what, name)
-    #     if signature:  # modules, attributes, ... don't have a signature
-    #         directive += signature
-    #     NAMES.append(name)
-    #     rst = directive + '\n\n' + doc_indent + obj.__doc__ + '\n'
-    #     DIRECTIVES[name] = rst
-
-    # def write_new_docs(app, exception):
-    #     txt = ['My module documentation']
-    #     txt.append('-----------------------\n')
-    #     for name in NAMES:
-    #         txt.append(DIRECTIVES[name])
-    #     # print('\n'.join(txt))
-    #     with open('../doc_new/generated.rst', 'w') as outfile:
-    #         outfile.write('\n'.join(txt))
-
-    # app.connect('autodoc-process-signature', get_rst)
-    # app.connect('build-finished', write_new_docs)
-
 autoclass_content = 'both'
 
 doxylink = {
-        'ct': (os.path.abspath('../../build/docs/Cantera.tag'),
-               '../../doxygen/html/')
+    'ct': (os.path.abspath('../../build/docs/Cantera.tag'),
+            '../../doxygen/html/')
 }
 
 intersphinx_mapping = {
     'python': ('https://docs.python.org/3', None),
     'pandas': ('https://pandas.pydata.org/pandas-docs/stable/', None),
     'numpy': ('https://numpy.org/doc/stable/', None),
+    'pint': ('https://pint.readthedocs.io/en/stable/', None),
 }
 
 # Ensure that the primary domain is the Python domain, since we've added the

diff --git a/doc/sphinx/cython/units.rst b/doc/sphinx/cython/units.rst
@@ -3,10 +3,17 @@
 Python Interface With Units
 ===========================
 
+This interface allows users to specify physical units associated with quantities.
+To do so, this interface leverages the `pint <https://pint.readthedocs.io/en/stable/>`__
+library to provide consistent unit conversion.
+
+Solution with Units
+-------------------
+
 .. autoclass:: Solution
 
 PureFluid Phases With Units
-===========================
+---------------------------
 
 The following convenience classes are available to create `PureFluid <PureFluid>`
 objects with the indicated equation of state:

diff --git a/interfaces/cython/cantera/with_units/solution.py.in b/interfaces/cython/cantera/with_units/solution.py.in
@@ -17,6 +17,12 @@ Q_ = units.Quantity
 
 
 def copy_doc(method):
+    """Decorator to copy docstrings from related methods in upstream classes.
+
+    This decorator will copy the docstring from the same named method in the upstream
+    class, either `Solution` or `PureFluid`. The docstring in the method being
+    decorated is appended to the upstream documentation.
+    """
     doc = getattr(method, "__doc__", None) or ""
     if isinstance(method, property):
         method = method.fget
@@ -32,10 +38,6 @@ def copy_doc(method):
 
 class Solution:
     """
-    A class for chemically-reacting solutions. Instances can be created to
-    represent any type of solution -- a mixture of gases, a liquid solution, or
-    a solid solution, for example.
-
     This implementation of `Solution <cantera.with_units.Solution>` operates with
     units by using the `pint` library to convert between unit systems. All properties
     are assigned units in the standard MKS system that Cantera uses, substituting kmol
@@ -50,51 +52,19 @@ class Solution:
     See the `pint documentation <https://pint.readthedocs.io>`__ for more information
     about using pint's ``Quantity`` classes.
 
-    The most common way to instantiate `Solution` objects is by using a phase
-    definition, species and reactions defined in an input file::
-
-        gas = ct.Solution('gri30.yaml')
-
-    If an input file defines multiple phases, the corresponding key in the
-    ``phases`` map can be used to specify the desired phase via the ``name`` keyword
-    argument of the constructor::
-
-        gas = ct.Solution('diamond.yaml', name='gas')
-        diamond = ct.Solution('diamond.yaml', name='diamond')
-
-    The name of the `Solution` object defaults to the *phase* identifier
-    specified in the input file. Upon initialization of a `Solution` object,
-    a custom name can assigned via::
-
-        gas.name = 'my_custom_name'
-
-    In addition, `Solution` objects can be constructed by passing the text of
-    the YAML phase definition in directly, using the ``yaml`` keyword
-    argument::
-
-        yaml_def = '''
-        phases:
-        - name: gas
-          thermo: ideal-gas
-          kinetics: gas
-          elements: [O, H, Ar]
-          species:
-          - gri30.yaml/species: all
-          reactions:
-          - gri30.yaml/reactions: declared-species
-          skip-undeclared-elements: true
-          skip-undeclared-third-bodies: true
-          state: {T: 300, P: 1 atm}
-        '''
-        gas = ct.Solution(yaml=yaml_def)
+    **Note:** This class is experimental. It only implements methods from `ThermoPhase`.
+    Methods from other classes are not yet supported. If you are interested in contributing
+    to this feature, please chime in on our enhancements issue:
+    `<https://github.com/Cantera/enhancements/issues/174>`__.
     """
     def __init__(self, infile="", name="", *, yaml=None):
         self.__dict__["_phase"] = _Solution(infile, name, yaml=yaml)
 
-    @common_properties@
+@common_properties@
 
-    @thermophase_properties@
+@thermophase_properties@
 
+Solution.__doc__ = f"{Solution.__doc__}\n{_Solution.__doc__}"
 
 class PureFluid:
     """
@@ -113,9 +83,9 @@ class PureFluid:
     about using pint's ``Quantity`` classes.
     """
     def __init__(self, infile, name="", *, yaml=None, **kwargs):
-        self.__dict__["_phase"] = _PureFluid(infile, name, **kwargs)
+        self.__dict__["_phase"] = _PureFluid(infile, name, yaml=yaml, **kwargs)
 
-    @common_properties@
+@common_properties@
 
     @property
     @copy_doc
@@ -135,12 +105,12 @@ class PureFluid:
                         f"Value {value!r} must be an instance of a pint.Quantity class"
                     ) from None
                 else:
-                    raise  # pragma: no cover
+                    raise
         else:
             Q = self.Q.magnitude
         self._phase.Q = Q
 
-    @thermophase_properties@
+@thermophase_properties@
 
     @property
     @copy_doc
@@ -150,7 +120,6 @@ class PureFluid:
 
     @TPQ.setter
     def TPQ(self, value):
-        msg = "Value {value!r} must be an instance of a pint.Quantity class"
         T = value[0] if value[0] is not None else self.T
         P = value[1] if value[1] is not None else self.P
         Q = value[2] if value[2] is not None else self.Q
@@ -159,12 +128,14 @@ class PureFluid:
                 val.ito(unit)
             except AttributeError as e:
                 if "'ito'" in str(e):
-                    raise CanteraError(msg.format(value=val)) from None
+                    raise CanteraError(
+                        f"Value {val!r} must be an instance of a pint.Quantity class"
+                    ) from None
                 else:
-                    raise  # pragma: no cover
+                    raise
         self._phase.TPQ = T.magnitude, P.magnitude, Q.magnitude
 
-    @purefluid_properties@
+@purefluid_properties@
 
 
 PureFluid.__doc__ = f"{PureFluid.__doc__}\n{_PureFluid.__doc__}"

diff --git a/samples/python/thermo/isentropic_units.py b/samples/python/thermo/isentropic_units.py
@@ -1,5 +1,7 @@
 """
-Isentropic, adiabatic flow example - calculate area ratio vs. Mach number curve
+Isentropic, adiabatic flow example - calculate area ratio vs. Mach number curve.
+Uses the pint library to include customized units in the calculation.
+
 
 Requires: Cantera >= 3.0.0, pint
 Keywords: thermodynamics, compressible flow, units
@@ -10,16 +12,16 @@
 
 # This sets the default output format of the units to have 2 significant digits
 # and the units are printed with a Unicode font. See:
-# https://pint.readthedocs.io/en/stable/formatting.html#unit-format-types
+# https://pint.readthedocs.io/en/stable/user/formatting.html
 ctu.units.default_format = ".2F~P"
 
 
 def soundspeed(gas):
     """The speed of sound. Assumes an ideal gas."""
 
     gamma = gas.cp / gas.cv
-    return np.sqrt(gamma * ctu.units.molar_gas_constant
-                     * gas.T / gas.mean_molecular_weight).to("m/s")
+    specific_gas_constant = ctu.units.molar_gas_constant / gas.mean_molecular_weight
+    return np.sqrt(gamma * specific_gas_constant * gas.T).to("m/s")
 
 
 def isentropic(gas=None):
@@ -45,7 +47,7 @@ def isentropic(gas=None):
     data = []
 
     # compute values for a range of pressure ratios
-    p_range = np.logspace(-3, 0, 200) * p0
+    p_range = np.logspace(-3, 0, 10) * p0
     for p in p_range:
 
         # set the state using (p,s0)

diff --git a/samples/python/thermo/sound_speed_units.py b/samples/python/thermo/sound_speed_units.py
@@ -1,5 +1,6 @@
 """
-Compute the "equilibrium" and "frozen" sound speeds for a gas
+Compute the "equilibrium" and "frozen" sound speeds for a gas. Uses the pint library to
+include customized units in the calculation.
 
 Requires: Cantera >= 3.0.0, pint
 Keywords: thermodynamics, equilibrium, units
@@ -57,7 +58,7 @@ def equilibrium_sound_speeds(gas, rtol=1.0e-6, max_iter=5000):
 if __name__ == "__main__":
     gas = ctu.Solution('gri30.yaml')
     gas.X = 'CH4:1.00, O2:2.0, N2:7.52'
-    T_range = np.linspace(80.33, 4760.33, 50) * ctu.units.degF
+    T_range = np.linspace(80, 4880, 25) * ctu.units.degF
     print("Temperature      Equilibrium Sound Speed     Frozen Sound Speed      Frozen Sound Speed Check")
     for T in T_range:
         gas.TP = T, 1.0 * ctu.units.atm