pyro-kinetics / pyrokinetics

Python library to run and analyse gyrokinetics simulations
https://pyrokinetics.readthedocs.io/en/latest/#
GNU Lesser General Public License v3.0
25 stars 6 forks source link

More explicit arguments for `LocalGeometry` and `LocalSpecies` #336

Open ZedThree opened 6 months ago

ZedThree commented 6 months ago

Currently, the constructors for both these classes (and their subclasses) are all just:

def __init__(self, *args, **kwargs):

This makes it difficult to do several things:

  1. understand what the input arguments are
  2. understand what the default values are
  3. construct these classes directly
  4. ensure everything is set

All this leads to some surprising behaviour.

For example, for LocalGeometry, some parameters aren't set by default:

mxh = LocalGeometryMXH()
hasattr(mxh, "kappa")
# False

and passing keyword arguments doesn't actually set them:

mxh = LocalGeometryMXH(kappa=1)
hasattr(mxh, "kappa")
# False

Passing a dict of values does set them, but then doesn't set anything else:

mxh = LocalGeometryMXH({"kappa":1})
print(mxh.__dict__)
# {'_already_warned': False, 'kappa': 1}

Having explicit arguments for these classes, with inline default values rather than in a separate function, will make these easier and less surprising to use. I think this will only break a few places that pass dicts to the constructor, e.g. from_gk_data, but this is an easy fix:

modified   src/pyrokinetics/local_geometry/local_geometry.py
@@ -285,7 +285,7 @@ class LocalGeometry:
         # be possible to have a local_geometry object that does not contain all attributes.
         # bunit_over_b0 should be an optional argument, and the following should
         # be performed within __init__ if it is None
-        local_geometry = cls(params)
+        local_geometry = cls(**params)

         # Values are not yet normalised
         local_geometry.bunit_over_b0 = local_geometry.get_bunit_over_b0()