Skip to content
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

add the 'name' attribute to dataframes that go through apply_frame_axis0 #15062

Conversation

llllllllll
Copy link
Contributor

  • closes #xxxx
  • tests added / passed
  • passes git diff upstream/master | flake8 --diff
  • whatsnew entry

Previously, if you did group.name in the applied function, it would fail and fall back to the slower path because the attribute did not exist.

shape_before was unused.

@codecov-io
Copy link

codecov-io commented Jan 5, 2017

Current coverage is 85.97% (diff: 100%)

Merging #15062 into master will decrease coverage by <.01%

@@             master     #15062   diff @@
==========================================
  Files           140        140          
  Lines         51263      51263          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          44076      44075     -1   
- Misses         7187       7188     +1   
  Partials          0          0          

Powered by Codecov. Last update be3f2ae...722a945

@sinhrks sinhrks added Groupby Performance Memory or execution speed performance labels Jan 5, 2017
@jreback
Copy link
Contributor

jreback commented Jan 9, 2017

looks innocuous enough (famous last words).

did you have a higher level usage that was triggering this?

@jreback jreback added this to the 0.20.0 milestone Jan 9, 2017
@jreback
Copy link
Contributor

jreback commented Jan 9, 2017

can you add an asv for this?

and add a whatsnew note (perf improvements).

@jreback
Copy link
Contributor

jreback commented Jan 21, 2017

ping @llllllllll

@llllllllll
Copy link
Contributor Author

Sorry about letting this sit idle. I will try to get the asv report and what's new on Monday.

@jreback
Copy link
Contributor

jreback commented Jan 23, 2017

np, ping when ready.

@llllllllll llllllllll force-pushed the add-name-in-apply-inference-call branch from dbb3a4a to 722a945 Compare January 23, 2017 21:25
@llllllllll
Copy link
Contributor Author

ping: I ran the suite locally but it was pretty noisy. I won't be run this without having other stuff running in the background until sometime tonight.

@jreback
Copy link
Contributor

jreback commented Jan 23, 2017

@llllllllll lgtm.

just report on perf when you can.

I also think that the .name arg is somewhat undocumented when passed to .apply, if you want to add a note there (or small example), might be useful.

@jreback
Copy link
Contributor

jreback commented Mar 23, 2017

@llllllllll can you rebase; ping on green. i think this was ok.

@llllllllll
Copy link
Contributor Author

from what I remember I was going to add a performance case for the asv suite. I had one locally but I actually saw a performance regression and didn't have time to dig deeper. I can rebase but I am not sure if we want to merge with the open question.

@jreback
Copy link
Contributor

jreback commented Mar 23, 2017

@llllllllll looks like you add an example in the asv. if you can rebase and just show/before after on that example would be fine.

@jreback jreback closed this in 8c8dd88 Mar 25, 2017
@jreback
Copy link
Contributor

jreback commented Mar 25, 2017

thanks @llllllllll rebased and merged.

mattip pushed a commit to mattip/pandas that referenced this pull request Apr 3, 2017
…ame_axis0

Previously, if you did `group.name` in the applied function, it would
fail and fall back to the slower path because the attribute did not
exist; `shape_before` was unused.

Author: Joe Jevnik <joe@quantopian.com>

This patch had conflicts when merged, resolved by
Committer: Jeff Reback <jeff@reback.net>

Closes pandas-dev#15062 from llllllllll/add-name-in-apply-inference-call and squashes the following commits:

722a945 [Joe Jevnik] DOC: update whatsnew for groupby perf change
7e75635 [Joe Jevnik] DEV: add groupby asv benchmark
710528a [Joe Jevnik] BUG: add the 'name' attribute to dataframes that go through apply_frame_axis0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Groupby Performance Memory or execution speed performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants