Skip to content

PERF: Improve replace perf#12745

Closed
sinhrks wants to merge 1 commit intopandas-dev:masterfrom
sinhrks:replace_perf
Closed

PERF: Improve replace perf#12745
sinhrks wants to merge 1 commit intopandas-dev:masterfrom
sinhrks:replace_perf

Conversation

@sinhrks
Copy link
Member

@sinhrks sinhrks commented Mar 30, 2016

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

When .replace is called with dict, replacements are done per value. Current impl try to soft convert the dtype in every replacement, but it is enough to be done in the final replacement.

Bench

-  712.83ms   355.93ms      0.50  replace.replace_convert.time_replace_series_timestamp
-     1.50s   698.21ms      0.46  replace.replace_convert.time_replace_frame_timestamp
-     3.12s   690.48ms      0.22  replace.replace_convert.time_replace_frame_timedelta
-     1.69s   354.83ms      0.21  replace.replace_convert.time_replace_series_timedelta
@sinhrks sinhrks added the Performance Memory or execution speed performance label Mar 30, 2016
@sinhrks sinhrks added this to the 0.18.1 milestone Mar 30, 2016
@jreback
Copy link
Contributor

jreback commented Mar 30, 2016

notice that BoolBlock.replace doesn't have convert arg at all! so not tested / working. can you give a check.
@sinhrks sinhrks force-pushed the replace_perf branch 2 times, most recently from 48637d9 to 31ae2e3 Compare March 30, 2016 14:40
@sinhrks
Copy link
Member Author

sinhrks commented Mar 30, 2016

Ah, found some .replace() conversion looks buggy. I'll submit a separate issue as the fix not looks straightforward.
jreback
jreback reviewed Mar 30, 2016
View reviewed changes
pandas/tests/series/test_replace.py Outdated
Copy link
Contributor

@jreback jreback Mar 30, 2016

Choose a reason for hiding this comment

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

great!
@sinhrks
Copy link
Member Author

sinhrks commented Mar 30, 2016

See #12747. Change this PR title to WIP until #12747 is fixed.
@sinhrks sinhrks changed the title PERF: Improve replace perf Mar 30, 2016
@sinhrks sinhrks force-pushed the replace_perf branch 2 times, most recently from dcf2ace to 904d4e1 Compare April 10, 2016 20:05
@jreback jreback removed this from the 0.18.1 milestone Apr 17, 2016
@codecov-io
Copy link

codecov-io commented May 21, 2016 •
edited
Loading

Current coverage is 85.27% (diff: 90.00%)

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

@@             master     #12745   diff @@
==========================================
  Files           144        144          
  Lines         50915      50921     +6   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43419      43423     +4   
- Misses         7496       7498     +2   
  Partials          0          0

Powered by Codecov. Last update dfeae39...ffc59b0
@jreback
Copy link
Contributor

jreback commented Sep 9, 2016

can you rebase / update?
@sinhrks sinhrks force-pushed the replace_perf branch 2 times, most recently from db1e776 to e16a99f Compare November 21, 2016 22:21
@jreback jreback added the Dtype Conversions Unexpected or buggy dtype conversions label Nov 23, 2016
@jreback
Copy link
Contributor

jreback commented Nov 23, 2016

lgtm. can we merge this independent of the other PR's? (so it could go in 0.19.2)
@sinhrks sinhrks changed the title WIP: PERF: Improve replace perf Nov 29, 2016
@sinhrks
Copy link
Member Author

sinhrks commented Nov 29, 2016 •
edited
Loading

It is independent. #12780 fixes some existing bugs separately.
@jreback jreback added this to the 0.19.2 milestone Nov 30, 2016
@jreback jreback closed this in e299560 Nov 30, 2016
@jreback
Copy link
Contributor

jreback commented Nov 30, 2016

thanks!
@sinhrks sinhrks deleted the replace_perf branch November 30, 2016 12:20
jorisvandenbossche pushed a commit that referenced this pull request Dec 15, 2016
@sinhrks @jorisvandenbossche
[Backport #12745] PERF: Improve replace perf
7479d41 
When .replace is called with
`dict`, replacements are done per value. Current impl try to soft
convert the dtype in every replacement, but it is enough to be done in
the final replacement.

Author: sinhrks <sinhrks@gmail.com>

Closes #12745 from sinhrks/replace_perf and squashes the following commits:

ffc59b0 [sinhrks] PERF: Improve replace perf

(cherry picked from commit e299560)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Dtype Conversions Unexpected or buggy dtype conversions Performance Memory or execution speed performance

3 participants

@sinhrks @jreback @codecov-io