Add var, std, median
#3378
Add var, std, median
#3378
Conversation
jl-wynen
force-pushed
the
var-std-median
branch
from
86a8aa0
to
ead950e
Compare
src/scipp/core/data_group.py
Outdated
There was a problem hiding this comment.
Should probably update
scipp/tests/data_group_test.py
Lines 826 to 837 in 8e19ec9
| } | ||
| ) | ||
| if isinstance(x, DataGroup): | ||
| return data_group_nary(sc_func, x, dim=dim, **kwargs) |
There was a problem hiding this comment.
Why sc_func and not _reduce_with_numpy with the same args used above for dataset?
There was a problem hiding this comment.
Because it's shorter. I could actually do the same for Dataset. It has a tiny performance penalty put that shouldn't matter.
| ddof: | ||
| 'Delta degrees of freedom'. | ||
| For sample variances, set ``ddof=1`` to obtain an unbiased estimator. | ||
| For normally distributed variables, set ``ddof=0`` to obtain a maximum | ||
| likelihood estimate. | ||
| See :func:`numpy.var` for more details. |
There was a problem hiding this comment.
Do you want to add a comment (or even a specific error?) why this is required by Scipp? Otherwise we risk a future developer coming and changing this.
src/scipp/core/reduction.py
Outdated
| This function computes the standard deviation of the input values which is *not* | ||
| the related to the ``x.variances`` property but instead defined as |
src/scipp/core/reduction.py
Outdated
| This function computes the standard deviation of the input values which is *not* | ||
| the related to the ``x.variances`` property but instead defined as |
| # Yes, using identical with floats. | ||
| # It works and allclose doesn't support datasets and data groups. | ||
| sc.testing.assert_identical( | ||
| sc.var(x, 'xx', ddof=0), |
There was a problem hiding this comment.
I suggest to add separate tests to ensure that ddof is required, to prevent changes by a future developer not aware of our intentions.
jl-wynen
force-pushed
the
var-std-median
branch
from
562ea74
to
92aad0d
Compare
Fixes #3357