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

Different results in V0.9.14 than in V0.9.16 #60

Closed
josephnowak opened this issue May 30, 2022 · 1 comment · Fixed by #61
Closed

Different results in V0.9.14 than in V0.9.16 #60

josephnowak opened this issue May 30, 2022 · 1 comment · Fixed by #61

Comments

@josephnowak
Copy link

josephnowak commented May 30, 2022

Hi, I was using this repo for a personal project and I decide to update the version and then I notice that the results of groupby aggregate function are totally different (I think that they are incorrect in the new version), here is an example:

import numpy_groupies as npg
import numpy as np
x = np.array([
    [ 1.,  2.],
    [ 4.,  4.],
    [ 5.,  2.],
    [np.nan,  3.],
    [ 8.,  7.]]
)
group_idx = np.array([1, 2, 2, 0, 1])
func = 'nanmax'
fill_value = np.nan
axis=0
npg.aggregate(group_idx, x, axis=axis, func=func, fill_value=fill_value)

# Result with V0.9.14, which I think is correct, because it match with the Xarray code in the last part
array([[nan,  3.],
       [ 8.,  7.],
       [ 5.,  4.]])

# Result with V0.9.16, you can notice that both results contain the same values, but this in particular has a different order
array([[nan,  8.],
       [ 5.,  3.],
       [ 7.,  4.]])
# The expected result can be obtained using Xarray groupby, which is the same than in the V0.9.14
import xarray as xr

xr.DataArray(
    x,
    coords={
        'a': [0, 1, 2, 3, 4],
        'b': [0, 1]
    }
).groupby(
    xr.DataArray(
        group_idx,
        dims=['a']
    )
).max()
@josephnowak josephnowak changed the title Different results from V0.9.14 and V0.9.16 Different results in V0.9.14 than in V0.9.16 May 30, 2022
@dcherian
Copy link
Collaborator

Thanks @josephnowak I've reverted the change that broke this in #61 since it isn't clear how to do it properly with the new method.

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