Add basic Experiment class #54
Conversation
henrikjacobsenfys
added
the
[scope] enhancement
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #54 +/- ##
============================================
+ Coverage 0.00% 61.53% +61.53%
============================================
Files 1 2 +1
Lines 2 117 +115
============================================
+ Hits 0 72 +72
- Misses 2 45 +43
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| import scipp as sc | ||
| import plopp as pp | ||
|
|
||
| from IPython.display import display |
There was a problem hiding this comment.
Your library should not import IPython - you should be able to run it from a python file or with a GUI - both of which don't need IPython.
| data = self._data.get(name) | ||
| if data is None: | ||
| raise ValueError(f"No data found for name: {name}") | ||
| fig = pp.plot(data.transpose(), title=f"{name}") |
There was a problem hiding this comment.
This should be hidden behind a check for running in the notebook mode.
| # Dunder methods | ||
| def __getitem__(self, key: str): | ||
| """Allow dictionary-style access: my_exp['vanadium']""" | ||
| return self._data[key] |
There was a problem hiding this comment.
It would be prudent to add checks on key. Otherwise you need to try..except in the caller, which is suboptimal
|
|
||
| def __setitem__(self, key: str, value: sc.DataArray): | ||
| """Allow dictionary-style setting: my_exp['vanadium'] = data""" | ||
| self._data[key] = value |
There was a problem hiding this comment.
same here - add some key checks
| for name, data in self._data.items(): | ||
| sc.io.save_hdf5(data, os.path.join(folder, f"{name}.h5")) | ||
|
|
||
| def append_data(self, new_data: sc.DataArray, name: str): |
There was a problem hiding this comment.
this method duplicates the functionality of __setitem__. You could use either one inside another to avoid duplication
|
|
||
| # TODO: Add checks of dimensions etc. I'm not yet sure what dimensions I want to allow, so for now I trust myself. | ||
|
|
||
| self.append_data(sc.io.load_hdf5(filename), name) |
There was a problem hiding this comment.
My linter swears at sc.io since they don't import io into scipp's __init__. Maybe the import is supposed be something like from scipp.io import load_hdf5?
| """ | ||
| super().__init__(name) | ||
| self._data = None | ||
| self._data = {} # store data as {name: DataArray} |
There was a problem hiding this comment.
you can just typehint the attribute like self._data: Dict[str, sc.DataArray] = {}
| Load data from an HDF5 file. | ||
|
|
||
| Args: | ||
| file_path (str): Path to the data file. |
| f"Available datasets: {list(self._data.keys())}" | ||
| ) | ||
|
|
||
| sc.io.save_hdf5(self._data[name], filename) |
There was a problem hiding this comment.
do you think it makes sense to mkdirs the path to filename here as well as in save_all_hdf5?
|
|
||
| self.append_data(sc.io.load_hdf5(filename), name) | ||
|
|
||
| def save_hdf5(self, name: str, filename: str): |
There was a problem hiding this comment.
Does it make sense to make name optional to follow the same approach as in save_all_hdf5?
| ) | ||
| self._data[name] = new_data | ||
|
|
||
| def get_data(self, name: str = None): |
There was a problem hiding this comment.
name typehint has to be optional here and in other methods with default None value
3 participants
It will need more methods for interacting with the data, such as plotting, binning and masking, but those are coming later.