Open BSchilperoort opened 1 year ago
Sounds very reasonable! All for keeping the codebase manageable.
To what extent do we actually need the DataStore class as subclass of Xarray's Dataset? What about instead of subclassing to add a dts accessor that holds the calibration functions? https://docs.xarray.dev/en/stable/internals/extending-xarray.html
ds.dts.single_ended_calibration()
ds.dts.double_ended_calibration()
ds.dts.sections
and its values are still stored in ds.attrs["_sections"]ds.dts.variance_stokes()
ds.dts.__repr__()
Taking your code it would look as follows:
from dtscalibration.calibration_routines import double_ended, single_ended
@xr.register_dataset_accessor("dts")
class DataStore:
...
def calibration_single_ended(self, **kwargs):
single_ended.calibrate(self, **kwargs)
def calibration_double_ended(self, **kwargs):
double_ended.calibrate(self, **kwargs)
This entire overhaul is much bigger than what you propose.. Maybe leave it at your proposal for now. I am doing this thing with the accessor with pandas object and it works really neat! Keeps everything tidy and allows for combining different accessors simultaneously.
The
datastore.py
module has become extremely large (6000 lines 😵), and has all the DataStore class methods explicitly implemented in the file. This makes development more difficult.One relatively easy way to improve this, is to split of the method implementations into different files. For example, make a new file
calibration_routines/double_ended.py
, and refer to this in the DataStore implementation: