Experiment mwp #90
Experiment mwp #90
Conversation
henrikjacobsenfys
added
[scope] enhancement
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #90 +/- ##
==========================================
Coverage ? 97.50%
==========================================
Files ? 30
Lines ? 1605
Branches ? 268
==========================================
Hits ? 1565
Misses ? 22
Partials ? 18
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:
|
AndrewSazonov
left a comment
AndrewSazonov
left a comment
There was a problem hiding this comment.
Small suggestion: it would be nice to update the placeholder markdown header cells to use the notebook title and short description I added in the previous PR (and also add one to the new experiment.ipynb). That way the code cells can automatically stretch to the full page width.
| a model of diffusion | ||
| - [Sample model](sample_model.ipynb) – Learn how to create a model of | ||
| the scattering from your sample | ||
| - [Experiment] (experiment.ipyng) - Learn how to load and bin your data |
There was a problem hiding this comment.
| - [Experiment] (experiment.ipyng) - Learn how to load and bin your data | |
| - [Experiment](experiment.ipyng) - Learn how to load and bin your data |
| value = int(value) | ||
| # This line can be removed when scipp resize support | ||
| # resizing with coordinates | ||
| dimensions[dim] = value |
There was a problem hiding this comment.
You're modifying dimensions in-place, while iterating over its items. This can lead to BAD things. Consider creating a copy of dimensions before and iterating over the copy (or the other way around)
| """Get the Q values from the dataset.""" | ||
| if self._data is None: | ||
| warnings.warn('No data loaded.', UserWarning) | ||
| return None |
There was a problem hiding this comment.
None is not the advertised sc.Variable. Consider changing -> sc.Variable to sc.Variable | None
| if self._data is None: | ||
| raise ValueError('No data to save.') | ||
|
|
||
| import os |
| 'giving the Experiment an invalid name' | ||
| # WHEN / THEN EXPECT | ||
| with pytest.raises(TypeError): | ||
| experiment.load_hdf5('some_file.h5', name=123) |
There was a problem hiding this comment.
Shouldn't the argument be display_name?
| a model of diffusion | ||
| - [Sample model](sample_model.ipynb) – Learn how to create a model of | ||
| the scattering from your sample | ||
| - [Experiment] (experiment.ipyng) - Learn how to load and bin your data |
There was a problem hiding this comment.
This should read experiment.ipynb
| import plopp as pp | ||
| import scipp as sc | ||
|
|
||
| # from easyscience.job.experiment import ExperimentBase |
| ########### | ||
|
|
||
| @staticmethod | ||
| def _in_notebook(): |
There was a problem hiding this comment.
this method should probably be typed to return -> bool
Add experiment class and minimal functionality