BUG: Groupby filter maintains ordering, closes #4621

[danielballan]

Simply adds np.sort.

Includes tests for SeriesGroupBy and DataFrameGroupBy, using indexes that are sequentially increasing, sequentially decreasing, and shuffled. Also, this PR removes all ad hoc sorting involved in the original filter tests -- now no longer needed. Much better!

[jreback]

looks good
in theory u might want to have a kw
sort=True that you would accept in order to turn this off
(only would matter with. large number of groups though)

[jreback]

can u add a release notes entry as well

[danielballan]

I'll add the sort keyword. Does that qualify as an API change, or is it too minor?

[jreback]

pretty minor - just make sure doc string is good

[danielballan]

Ready (assuming Travis agrees!)

[hayd]

This looks good, though I dislike the use of sort kwarg to mean "maintains original order"... not sure I have alternative.

[jtratner]

Yes, I agree - that's very confusing. Is the alternative not sorting it?

[jreback]

sort is the kw in groupby already so it's consistent (or maybe it's sorted?)
I don't remember u may be able to just use that kw

[hayd]

@jreback that's a good point, I'd not seen or used that.

sort : boolean, default True
Sort group keys. Get better performance by turning this off

Although this is somewhat different, in that case it is actually sorting the keys. Here we are putting them back into their previous order of the index (and this may - or may not - be sorted)...

[danielballan]

Exactly. IMO, there is a strong case for not even allowing the option to skip the sorting. For one thing, it's difficult to test, which seems like a red flag. (My original tests sorted the result before comparing to the expected one.)

[jreback]

this is the indexer it should always be sorted
just skip a keyword on this

it should always be in the same order as the groups which is why it needs sorting, because the groups are sorted (groupsort_indexer)

[danielballan]

Removed sort, including docstring and release notes. Ready.