Skip to content
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

Mistaken use of ds.var() in core.py? #37

Closed
jrising opened this issue Aug 23, 2022 · 3 comments · Fixed by #41
Closed

Mistaken use of ds.var() in core.py? #37

jrising opened this issue Aug 23, 2022 · 3 comments · Fixed by #41

Comments

@jrising
Copy link

jrising commented Aug 23, 2022

In core.py, there are a few loops of the form: for var in ds.var():.

This tries to compute a variance across all dimensions, for each variable. Is that the intention? I think you just mean for var in ds:.

Note that if any variables are of a type for which var cannot be computed (e.g., timedelta64[ns]) then aggregate fails.

@ks905383
Copy link
Owner

ks905383 commented Aug 23, 2022

Yikes, yeah that's definitely a bug. Weird that it hasn't triggered any of the tests yet...

Yeah, should likely be for var in ds:

Thanks for pointing this out, will take a look.

@ks905383
Copy link
Owner

ks905383 commented Sep 5, 2022

Ok thankfully it hasn't caused any computational problems, since for types for which ds.var() doesn't fail [var for var in ds.var()] returns the same thing as [var for var in ds] (with an additional computation time of course). But it definitely needs to get fixed - it'll be out in the next minor release.

ks905383 added a commit that referenced this issue Sep 5, 2022
replaces `for var in ds.var():` with `for var in ds:` in core
@jrising
Copy link
Author

jrising commented Sep 6, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants