Skip to content

Conversation

simonjayhawkins
Copy link
Member

Backport PR #38148

@simonjayhawkins simonjayhawkins added the Indexing Related to indexing on series/frames, not to indexes themselves label Nov 30, 2020
@simonjayhawkins simonjayhawkins added this to the 1.1.5 milestone Nov 30, 2020
@simonjayhawkins
Copy link
Member Author

@phofl #38148 (comment) was this necessary? do we know what commit caused the performance regression. the existing code on 1.1.x looks much simpler to what was replaced on master.

@phofl
Copy link
Member

phofl commented Nov 30, 2020

The underlying performance problems existed with this solution too. The loop is the actual problem, not the if else logic.

@phofl
Copy link
Member

phofl commented Nov 30, 2020

That is tricky. The tests are failing, because Index.union is different on master.

idx = pd.RangeIndex(0, 2)
result = idx.union([1])

This returns a RangeIndex on master and an Index with dtype="object" on 1.1.4.

Suggestions about how to proceed here?

cc @simonjayhawkins

@jorisvandenbossche
Copy link
Member

Since this was not a performance regression (at least, the issue #37954 does not mention anything about a regression, AFAIK), I don't think we should be backporting this.

@simonjayhawkins
Copy link
Member Author

Since this was not a performance regression (at least, the issue #37954 does not mention anything about a regression, AFAIK), I don't think we should be backporting this.

I agree.

closing this an can maybe revisit pending outcome of discussions in #38148

if not reverted the release note needs to be moved, but this is tracked by https://github.com/simonjayhawkins/pandas-release/actions/runs/391543194 so won't be missed.

@simonjayhawkins simonjayhawkins deleted the backport-of-pr-38148-on-1.1.x branch November 30, 2020 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Indexing Related to indexing on series/frames, not to indexes themselves
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants