Skip to content

Fix map(series) for unsorted base series index#5459

Merged
TomAugspurger merged 5 commits intodask:masterfrom
bluecoconut:mapseries-bugfix
Oct 7, 2019
Merged

Fix map(series) for unsorted base series index#5459
TomAugspurger merged 5 commits intodask:masterfrom
bluecoconut:mapseries-bugfix

Conversation

@bluecoconut
Copy link
Copy Markdown
Contributor

@bluecoconut bluecoconut commented Oct 3, 2019

Found a bug where tests were not actually catching this, fixes #5458

  • Tests added / passed
  • Passes black dask / flake8 dask

def mapseries_combine(index, concat_result):
final_series = concat_result.sort_index()
final_series.index = index
final_series = pd.Series(index, index=index).map(final_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.

We've been trying to avoid using pd.Series explicitly, in order to improve support for other dataframe libraries that are similar enough to pandas (notably RAPIDS cudf). Can you think of a way that would produce a Series object that don't explicitly use the pandas module?

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.

I'll work on this a bit and see what I can do. First few attempts somehow ended up passing tests, but functionally gave wrong results, so I'm gonna update some tests too to make this cleaner.

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.

Actually, just found that index.to_series() is nearly identical to pd.Series(index, index=index), so just use this to_series() method and everything works!

@TomAugspurger TomAugspurger merged commit 8c7f597 into dask:master Oct 7, 2019
@TomAugspurger
Copy link
Copy Markdown
Member

Thanks @bluecoconut.

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.

Bug in MapSeries for unsorted index base series

3 participants