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

Mode is returning unexpected results #13

Open
JaschaMuller opened this issue Dec 1, 2020 · 2 comments
Open

Mode is returning unexpected results #13

JaschaMuller opened this issue Dec 1, 2020 · 2 comments

Comments

@JaschaMuller
Copy link

Hi, I really like this package and it really optimizes some of my workflow.
I have however experience unexcepted results from the npi.group_by(b).mode(a) function.

This is your test code, which works fine (probably because the zones are in sorted format)

a = [4, 5, 6, 4, 0, 9, 8, 5, 4, 9] # Values
b = [0, 0, 0, 0, 1, 2, 2, 2, 2, 2] # Zones
k, u = npi.group_by(b).mode(a)
--> out u : array([4, 0, 9])

However as soon the the zones are not in a sorted format, the results of the mode is not correct. I am not sure if this is a requirement (sorted zones), since all the other statistical operations (mean mix max etc.) seem to work fine with unsorted zones.

a = [2,2,2,3,1,3,4,9,6,5] # Values
b = [1,1,1,2,1,2,2,3,4,5] # Zones
k, u = npi.group_by(b).mode(a)
--> out u : array([2, 1, 9, 6, 5]) 

where I am expecting the output to be [2,3,9,6,5]

The above example works correctly for the other statistic types.
I hope this makes sense, perhaps I have a wrong understanding or interpretation.
Thanks

@EelcoHoogendoorn
Copy link
Owner

Thanks for the input. Havnt done maintenance on this repo in ages, but I might find some time for it in the coming week. Looks indeed like a legit bug. Ill need to dig in to find the optimal solution. Indeed permuting the arrays with the argsort of b gives the correct result; so that could be one fix but im not sure its the best one. Also the returned k does not return what it says in the docstring, and what youd expect from the rest of the API. I think I threw in this functionality as a bonus without ever really being invested in its correctness; but hopefully this all can be fixed properly.

@JaschaMuller
Copy link
Author

Thanks a lot, I will meanwhile try the work-around that you suggested.

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

No branches or pull requests

2 participants