Pd.series.map performance
State: closed

There are other places also to refactor to improve the performance, but this current change has greater impact aswell.

- [ ] closes #34717
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry `improved the performance of pd.series.map`


Labels:
name: Performance
description: Memory or execution speed performance
name: Series
description: Series data structure
name: Constructors
description: Series/DataFrame/Index/pd.array Constructors

Comments:
Yeah, ``zip`` is surprisingly slow,  thought I think simply

```python
keys = tuple(data.keys())
values = tuple(data.values())
```
should be even faster than your approach. Can you compare this to your PR.

Can you also give perf. comparisons relative to master.
> Yeah, `zip` is surprisingly slow, thought I think simply
> 
> ```python
> keys = tuple(data.keys())
> values = tuple(data.values())
> ```
> 
> should be even faster than your approach. Can you compare this to your PR.
> 
> Can you also give perf. comparisons relative to master.

values = tuple(data.values()) --> this needs to be returned in the order of keys. If i use in the given way the tests are failing
what about using `list` instead of `tuple` for both, will it respect the order of keys?
It is kind of expecting keys as tuple and values as list. I can see tests passing. And values order should be in the same form as keys. In the sense data = {1:2,3:2,:4:5}, keys = tuple(3,4,1) and values = [2,5,2]. But anyhow i see if we try keys = tuple(data.keys()) and values = list(data.values()). This should be fine i think. Let me see and validate the tests
emm, why does keys have to be `tuple`? i just saw the `else` clause below your change in the file, where:
```python
keys, values = [], []
```
so maybe no need for keys to be tuple?

sorry for commenting this without looking at the codebase in details and apologize in advance if it has to be tuple.
If you look at the code history.
```
>keys,values = zip(*data.items())
> values = list(values)
```
If you look at the above piece of code. Zip returns tuples of keys and values. But in the second line values are being converted to list.  But i agree even in the else he is assigning empty list for both keys and values.
well, I _think_ it's fine to leave both to be `list`

but just follow your own idea to make the changes, and you might also need to add a whatsnew and an example in asv for performance comp.
> well, I _think_ it's fine to leave both to be `list`
> 
> but just follow your own idea to make the changes, and you might also need to add a whatsnew and an example in asv for performance comp.

Yeah. I will look into it and try to generate the perf comp accordingly.
@charlesdong1991 Any idea how to deal with the breaking `type validation`
emm, you meant CI? seem you have list changed to tuple. can you try using list for both and see if this error is gone?
Hello @Rohith295! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:



There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-09-13 15:32:24 UTC
> emm, you meant CI? seem you have list changed to tuple. can you try using list for both and see if this error is gone?

Alright. I will check it accordinly.
> but just follow your own idea to make the changes, and you might also need to add a whatsnew and an example in asv for performance comp.

I am trying to generate the performance metrics. Can you suggest me some practices for generating it properly?
https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#running-the-performance-test-suite

you can also add a test for this case in `asv_bench`.
> https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#running-the-performance-test-suite

Alright. I will have a look into this `asv`
> yeah pls provide an asv for this case and also run the construction asvs to make sure we don't have any regressions elsewhere

Alright. I will fix the review comments and do provide the asv
> values = tuple(data.values()) --> this needs to be returned in the order of keys. If i use in the given way the tests are failing

@Rohith295, is the failing test ``tests/series/methods/test_cov_corr.py::TestSeriesCorr::test_corr``? That one *fails* for me after I apply my proposal if I run the test suite using xdist, but *passes* if I run the tests sequentially or only run that individual test. This could hint that there is a problem with isolation of e.g. fixtures in the pandas test suite.

[The python docs](https://docs.python.org/3/library/stdtypes.html#dictionary-view-objects) says:

> keys and values are iterated over in insertion order.

so ``values = data.values()`` should work, AFAICS.
> > values = tuple(data.values()) --> this needs to be returned in the order of keys. If i use in the given way the tests are failing
> 
> @Rohith295, is the failing test ``tests/series/methods/test_cov_corr.py::TestSeriesCorr::test_corr``? That one *fails* for me after I apply my proposal if I run the test suite using xdist, but *passes* if I run the tests sequentially or only run that individual test. This could hint that there is a problem with isolation of e.g. fixtures in the pandas test suite.
> 
> [The python docs](https://docs.python.org/3/library/stdtypes.html#dictionary-view-objects) says:
> 
> > keys and values are iterated over in insertion order.
> 
> so ``values = data.values()`` should work, AFAICS.

It's not only that one particular test, there were other tests aswell. I did not make a particular note of it of tests which are failing. Also as you mentioned data.values() gives output in particular order. I still don't want rely on it because it only implemented from python 3.6, but what if someone using older versions. I believe our code must be both backward and forward compatible.
>  I still don't want rely on it because it only implemented from python 3.6, but what if someone using older versions

pandas supports python 3.6.1 and higher, so that particular concern is not a problem. But ok if tuple(data.values()) doesn't work currently, I'm fine with that issue being outside th scope of this PR.
we need an asv & a whatsnew note in order to merge this.
I will do it by today mostly. Sorry for the delay @jreback 
Guys, i am trying to build asv. If i start doing asv run. It is taking a long time by executing all the files under benchmarks directory. Do any one of you know just make the specific file execute under benchmarks directory to capture the asv file. @jreback  @topper-123 @charlesdong1991 
> if u can add a whats new note and an asv that captures this case
> 
> you can run just other relevant asvs (if we have any for map)
> 
> and show the results of this one
> 
> merge master and ping on green

Alright. I am on it..
· Creating environments
· Discovering benchmarks
·· Uninstalling from conda-py3.6-Cython0.29.16-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
·· Building 5dc5004e <pd.Series.map_performance> for conda-py3.6-Cython0.29.16-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
·· Installing 5dc5004e <pd.Series.map_performance> into conda-py3.6-Cython0.29.16-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
· Running 2 total benchmarks (2 commits * 1 environments * 1 benchmarks)
[  0.00%] · For pandas commit 04e9e0af <pd.Series.map_performance^2> (round 1/2):
[  0.00%] ·· Building for conda-py3.6-Cython0.29.16-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[  0.00%] ·· Benchmarking conda-py3.6-Cython0.29.16-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[ 25.00%] ··· Running (series_methods.Map.time_map--).
[ 25.00%] · For pandas commit 5dc5004e <pd.Series.map_performance> (round 1/2):
[ 25.00%] ·· Building for conda-py3.6-Cython0.29.16-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[ 25.00%] ·· Benchmarking conda-py3.6-Cython0.29.16-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[ 50.00%] ··· Running (series_methods.Map.time_map--).
[ 50.00%] · For pandas commit 5dc5004e <pd.Series.map_performance> (round 2/2):
[ 50.00%] ·· Benchmarking conda-py3.6-Cython0.29.16-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[ 75.00%] ··· series_methods.Map.time_map                                                                            ok
[ 75.00%] ··· ======== ============ ============ =============
              --                          a
              -------- ---------------------------------------
                 m        object      category        int
              ======== ============ ============ =============
                dict    3.20±0.1ms   1.95±0.1ms   1.72±0.03ms
               Series   2.27±0.1ms   936±100μs      728±90μs
               lambda   6.46±0.2ms   1.29±0.2ms     8.01±1ms
              ======== ============ ============ =============

[ 75.00%] · For pandas commit 04e9e0af <pd.Series.map_performance^2> (round 2/2):
[ 75.00%] ·· Building for conda-py3.6-Cython0.29.16-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[ 75.00%] ·· Benchmarking conda-py3.6-Cython0.29.16-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[100.00%] ··· series_methods.Map.time_map                                                                            ok
[100.00%] ··· ======== ============ ============= ============
              --                          a
              -------- ---------------------------------------
                 m        object       category       int
              ======== ============ ============= ============
                dict    3.87±0.6ms    2.44±0.2ms   2.93±0.2ms
               Series    3.42±1ms    1.01±0.07ms   1.07±0.5ms
               lambda   6.08±0.5ms    1.60±0.1ms    6.84±1ms
              ======== ============ ============= ============

       before           after         ratio
     [04e9e0af]       [5dc5004e]
     <pd.Series.map_performance^2>       <pd.Series.map_performance>
-      2.44±0.2ms       1.95±0.1ms     0.80  series_methods.Map.time_map('dict', 'category')
-      2.93±0.2ms      1.72±0.03ms     0.59  series_methods.Map.time_map('dict', 'int')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.
@jreback I added the asv benchmarking output here. Is this what expected?
@jreback yes we do have sufficient asv's. Also added the comments as requested and also the release note under perf section.
thanks @Rohith295 
Thanks @jreback . I have learned so much from your review comments and also got to chance to understand more about open source community.