Skip to content

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