-
Notifications
You must be signed in to change notification settings - Fork 62
Update mean and sum functions #643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 13 commits
dd69045
c5df2f0
f76a037
1e3914e
a675cd3
88e18b0
e3911ad
5ebe144
dda99f2
2b8422e
65d319a
ca46824
1c0bfc7
5f624bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changes to contributors should be made using the bot, please remove these files from the PR. |
||
<!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section --> | ||
[](#contributors-) | ||
[](#contributors-) | ||
<!-- ALL-CONTRIBUTORS-BADGE:END --> | ||
## Contributors ✨ | ||
|
||
|
@@ -76,7 +76,6 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d | |
<tr> | ||
<td align="center" valign="top" width="14.28%"><a href="https://github.com/psotom"><img src="https://avatars.githubusercontent.com/u/166627986?v=4?s=100" width="100px;" alt="psotom"/><br /><sub><b>psotom</b></sub></a><br /><a href="https://github.com/GAA-UAM/scikit-fda/commits?author=psotom" title="Code">💻</a> <a href="#example-psotom" title="Examples">💡</a> <a href="#ideas-psotom" title="Ideas, Planning, & Feedback">🤔</a> <a href="https://github.com/GAA-UAM/scikit-fda/commits?author=psotom" title="Tests">⚠️</a> <a href="#research-psotom" title="Research">🔬</a> <a href="https://github.com/GAA-UAM/scikit-fda/commits?author=psotom" title="Documentation">📖</a></td> | ||
<td align="center" valign="top" width="14.28%"><a href="https://github.com/luisheb"><img src="https://avatars.githubusercontent.com/u/24703335?v=4?s=100" width="100px;" alt="Luis Hebrero"/><br /><sub><b>Luis Hebrero</b></sub></a><br /><a href="https://github.com/GAA-UAM/scikit-fda/commits?author=luisheb" title="Documentation">📖</a></td> | ||
<td align="center" valign="top" width="14.28%"><a href="https://github.com/E105D104U125"><img src="https://avatars.githubusercontent.com/u/72515278?v=4?s=100" width="100px;" alt="E69D68U85"/><br /><sub><b>E69D68U85</b></sub></a><br /><a href="https://github.com/GAA-UAM/scikit-fda/commits?author=E105D104U125" title="Code">💻</a></td> | ||
</tr> | ||
</tbody> | ||
<tfoot> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -427,20 +427,61 @@ def sum( # noqa: WPS125 | |
""" | ||
super().sum(axis=axis, out=out, keepdims=keepdims, skipna=skipna) | ||
|
||
valid_functions = ~self.isna() | ||
valid_counts = np.sum(valid_functions, axis=0) | ||
valid_coefficients = self.coefficients[valid_functions] | ||
|
||
coefs = ( | ||
np.nansum(self.coefficients, axis=0) if skipna | ||
else np.sum(self.coefficients, axis=0) | ||
np.nansum(valid_coefficients, axis=0) if skipna | ||
else np.sum(valid_coefficients, axis=0) | ||
) | ||
|
||
if min_count > 0: | ||
valid = ~np.isnan(self.coefficients) | ||
n_valid = np.sum(valid, axis=0) | ||
coefs[n_valid < min_count] = np.nan | ||
|
||
coefs = np.where(valid_counts >= min_count, coefs, np.nan) | ||
|
||
return self.copy( | ||
coefficients=coefs, | ||
sample_names=(None,), | ||
) | ||
|
||
def mean( # noqa: WPS125 | ||
self: T, | ||
*, | ||
axis: Optional[int] = None, | ||
dtype: None = None, | ||
out: None = None, | ||
keepdims: bool = False, | ||
skipna: bool = False, | ||
min_count: int = 0, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems to me that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is left for compatibility with the mean functions of FDataIrregular and Grid, but it does not make sense to use it, as you do not have measurements for each observation, but simply the observations approximated by functions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would at least make sense in a global level (if some curves are NaN because they were not measured). Of course, if you only have a |
||
) -> T: | ||
"""Compute the mean of all the samples. | ||
|
||
Args: | ||
axis: Used for compatibility with numpy. Must be None or 0. | ||
dtype: Used for compatibility with numpy. Must be None. | ||
out: Used for compatibility with numpy. Must be None. | ||
keepdims: Used for compatibility with numpy. Must be False. | ||
skipna: Wether the NaNs are ignored or not. | ||
min_count: Number of valid (non NaN) data to have in order | ||
for the a variable to not be NaN when `skipna` is | ||
`True`. | ||
|
||
Returns: | ||
A FDataBasis object with just one sample representing | ||
the mean of all the samples in the original object. | ||
""" | ||
super().mean(axis=axis, dtype=dtype, out=out, keepdims=keepdims, | ||
aleexarias marked this conversation as resolved.
Show resolved
Hide resolved
|
||
skipna=skipna) | ||
|
||
return ( | ||
self.sum( | ||
axis=axis, | ||
out=out, | ||
keepdims=keepdims, | ||
skipna=skipna, | ||
min_count=min_count, | ||
) | ||
/ np.sum(~self.isna()), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the trailing comma. Otherwise, you are returning a tuple and the tests fail:
|
||
) | ||
|
||
def var( | ||
self: T, | ||
|
@@ -998,7 +1039,7 @@ def isna(self) -> NDArrayBool: | |
Returns: | ||
na_values (np.ndarray): Positions of NA. | ||
""" | ||
return np.all( # type: ignore[no-any-return] | ||
return np.any( # type: ignore[no-any-return] | ||
np.isnan(self.coefficients), | ||
axis=1, | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you removing a contributor??