Skip to content

Add to_series method#3613

Merged
mrocklin merged 4 commits intodask:masterfrom
henriqueribeiro:feature/index_to_series
Jun 15, 2018
Merged

Add to_series method#3613
mrocklin merged 4 commits intodask:masterfrom
henriqueribeiro:feature/index_to_series

Conversation

@henriqueribeiro
Copy link
Copy Markdown
Contributor

  • Tests added / passed
  • Passes flake8 dask

Adding method to_series() on dask.dataframe.Index class

Copy link
Copy Markdown
Member

@mrocklin mrocklin 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 great to me. Thanks for the fix @henriqueribeiro !

I left a small comment, but I'd be happy to merge this as-is.

@derived_from(pd.Index)
def to_series(self):
return self.map_partitions(M.to_series,
meta=self._meta.to_series())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My guess is that if you leave off the meta= keyword here that it will probably do the right thing anyway.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, I tried that also and it worked.

@henriqueribeiro
Copy link
Copy Markdown
Contributor Author

Good! Should I do something?

@mrocklin
Copy link
Copy Markdown
Member

Good! Should I do something?

If you want to remove the meta= keyword then you should do so and then push the change back up to this branch. Otherwise I'll just merge now.

@henriqueribeiro
Copy link
Copy Markdown
Contributor Author

According to the documentation:

This may lead to unexpected results, so providing meta is recommended.
So, maybe we should leave like this :)

@mrocklin mrocklin merged commit 8dadb1b into dask:master Jun 15, 2018
@mrocklin
Copy link
Copy Markdown
Member

In this particular case you would have been fine. But lets just merge this for now.

Thanks @henriqueribeiro !

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