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

array function implementation for np.cumprod / np.nancumprod fails with axis #939

Closed
keewis opened this issue Dec 19, 2019 · 2 comments · Fixed by #940
Closed

array function implementation for np.cumprod / np.nancumprod fails with axis #939

keewis opened this issue Dec 19, 2019 · 2 comments · Fixed by #940
Labels
numpy Numpy related bug/enhancement

Comments

@keewis
Copy link
Contributor

keewis commented Dec 19, 2019

Passing a axis argument to cumprod / nancumprod will make them fail. I tracked it down to

if pre_calc_units.dimensionless:
where pre_calc_units can be a string ("dimensionless"), but a Unit object was expected. The string was created when implement_func was called for cumprod and nancumprod (by the way, cumprod also gets implemented with set_units_ufuncs). We can fix this by passing None as input_units instead, but if feasible I'd prefer "dimensionless" since that is way easier to read and understand. For that, we might need to preprocess the input_units parameter in implement_func.

@hgrecco
Copy link
Owner

hgrecco commented Dec 19, 2019

I think is the same as #867 , right?

@hgrecco hgrecco added the numpy Numpy related bug/enhancement label Dec 19, 2019
@keewis
Copy link
Contributor Author

keewis commented Dec 19, 2019

#867 is about the units of the result if one were to pass axis (and existed before #905), while this issue was introduced in #905 when implement_func was written. cumprod is only defined for dimensionless, so the resulting unit is not a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
numpy Numpy related bug/enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants