Skip to content

minor sort_values housekeeping#7462

Merged
jsignell merged 5 commits intodask:mainfrom
celsiustx:sort-values
Mar 25, 2021
Merged

minor sort_values housekeeping#7462
jsignell merged 5 commits intodask:mainfrom
celsiustx:sort-values

Conversation

@ryan-williams
Copy link
Contributor

  • Closes #xxxx
  • Tests added / passed
  • Passes black dask / flake8 dask

a few mostly-cosmetic follow-ons to sort_values / #7286

return self[list(cs)]

def sort_values(self, other, npartitions=None, ascending=True, **kwargs):
def sort_values(self, by, npartitions=None, ascending=True, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just take the ascending=True out of here and out of sort_values below

Copy link
Member

@jsignell jsignell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, but while you are improving this I am wondering if we should do @derived_from(pd.DataFrame) instead.

@jsignell
Copy link
Member

I'm going to go ahead and merge this actually, but @ryan-williams if you feel inspired to keep tweaking please go for it!

@jsignell jsignell merged commit f7bb556 into dask:main Mar 25, 2021
@ryan-williams ryan-williams deleted the sort-values branch March 25, 2021 21:47
@ryan-williams
Copy link
Contributor Author

thanks @jsignell, I'll try to do another PR for that (also will try hand at Series.sort_values if I have time)

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 this pull request may close these issues.

2 participants