Skip to content

BUG: Segfault with string Index when using Rolling after Groupby #36753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 6, 2020

Conversation

phofl
Copy link
Member

@phofl phofl commented Sep 30, 2020

The segfault was caused by self.obj.index.asi8=None when Index is a string Index. self._on.asi8 solves that issue.

Additionally I noticed, that obj.index was already sorted, so the insert of extra_col mixed up the order. We should use self.obj.index.

I will add a whats new after #36689 is merged.

cc @mroeschke

@phofl phofl added Regression Functionality that used to work in a prior pandas version Window rolling, ewma, expanding labels Sep 30, 2020
@mroeschke
Copy link
Member

Thanks @phofl! I suspect we may need to be using self._on in a lot more places as I don't think our test coverage of the on keyword is really extensive

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

pls add a whatsnew note 1.1.4 for rolling (note that the whatsnew hasn't been pushed yet)

@jreback jreback added this to the 1.1.4 milestone Oct 1, 2020
@jreback
Copy link
Contributor

jreback commented Oct 1, 2020

hah you already mentioned

I will add a whats new after #36689 is merged.

@jorisvandenbossche jorisvandenbossche changed the title [BUG]: Segfault with string Index when using Rolling after Groupby BUG: Segfault with string Index when using Rolling after Groupby Oct 1, 2020
@simonjayhawkins
Copy link
Member

I will add a whats new after #36689 is merged.

@phofl

@jreback jreback merged commit 3a63fed into pandas-dev:master Oct 6, 2020
@jreback
Copy link
Contributor

jreback commented Oct 6, 2020

thanks @phofl very nice

@simonjayhawkins
Copy link
Member

needs a release note

@simonjayhawkins
Copy link
Member

@meeseeksdev backport 1.1.x

@lumberbot-app
Copy link

lumberbot-app bot commented Oct 6, 2020

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout 1.1.x
$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 3a63fedefe9c4f06c66242c07a211e1bbcd0f596
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #36753: BUG: Segfault with string Index when using Rolling after Groupby'
  1. Push to a named branch :
git push YOURFORK 1.1.x:auto-backport-of-pr-36753-on-1.1.x
  1. Create a PR against branch 1.1.x, I would have named this PR:

"Backport PR #36753 on branch 1.1.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

@phofl
Copy link
Member Author

phofl commented Oct 6, 2020

@jreback Thanks.

Sorry was busy today. Should I open another PR for a Release Note? cc @simonjayhawkins

simonjayhawkins pushed a commit to simonjayhawkins/pandas that referenced this pull request Oct 9, 2020
simonjayhawkins added a commit that referenced this pull request Oct 9, 2020
…hen using Rolling after Groupby (#37003)

Co-authored-by: patrick <[email protected]>
@phofl phofl deleted the 36727 branch October 10, 2020 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Regression Functionality that used to work in a prior pandas version Window rolling, ewma, expanding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Segmentation fault when doing pandas.core.window.rolling.RollingGroupBy.apply
4 participants