TYP #37715: Fix mypy errors in accessor.py #50005
Conversation
pandas/core/strings/accessor.py
Outdated
| # error: Incompatible types in assignment (expression has type | ||
| # "ndarray", variable has type "Series") | ||
| data = ensure_object(data) # type: ignore[assignment] | ||
| data = Series(ensure_object(data)) |
There was a problem hiding this comment.
Is this necessary code-wise? Creating a Series adds unnecessary overhead
There was a problem hiding this comment.
Yea that was my first thought too but the other code branch does work with a series. @bang128 can you just run the asv benchmarks for cat and post the results to make sure no regression?
There was a problem hiding this comment.
Not sure if we have coverage here, would probably be better to use a specific example with timeit
There was a problem hiding this comment.
Since it said that "expression has type "ndarray", variable has type "Series", I just converted the "ndarray" to "Series"
There was a problem hiding this comment.
@WillAyd What do you mean with run the asv benchmarks for cat?
There was a problem hiding this comment.
@WillAyd I apologize for my late response. It seems that I need conda to run asv benchmark, don't I? But I am facing a problem installing conda: even though I installed miniconda3, it still says that there is no conda in my machine. Actually, this is the first time I run asv benchmark, can you help me, please?
There was a problem hiding this comment.
My guess is you installed conda but maybe didn't run conda init to have it get activated by default by your shell.
You can alternately try changing asv_bench/asv.conf.json to set the environment_type to virtualenv instead of conda if you can't get the conda setup working
There was a problem hiding this comment.
@WillAyd Thank you for your recommendation. I ran asv benchmark successfully, and this is the result:
· Creating environments
· Discovering benchmarks
·· Uninstalling from virtualenv-py3.8-Cython0.29.32-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pyarrow-scipy-sqlalchemy-tables-xlrd-xlsxwriter..
·· Installing 6c677bc <Fix-#37715-accessor.py> into virtualenv-py3.8-Cython0.29.32-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pyarrow-scipy-sqlalchemy-tables-xlrd-xlsxwriter...
· Running 2 total benchmarks (2 commits * 1 environments * 1 benchmarks)
[ 0.00%] · For pandas commit bc987e7 (round 1/2):
[ 0.00%] ·· Building for virtualenv-py3.8-Cython0.29.32-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pyarrow-scipy-sqlalchemy-tables-xlrd-xlsxwriter....
[ 0.00%] ·· Benchmarking virtualenv-py3.8-Cython0.29.32-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pyarrow-scipy-sqlalchemy-tables-xlrd-xlsxwriter
[25.00%] ··· Running (strings.Cat.time_cat--).
[25.00%] · For pandas commit 6c677bc <Fix-#37715-accessor.py> (round 1/2):
[25.00%] ·· Building for virtualenv-py3.8-Cython0.29.32-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pyarrow-scipy-sqlalchemy-tables-xlrd-xlsxwriter....
[25.00%] ·· Benchmarking virtualenv-py3.8-Cython0.29.32-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pyarrow-scipy-sqlalchemy-tables-xlrd-xlsxwriter
[50.00%] ··· Running (strings.Cat.time_cat--).
[50.00%] · For pandas commit 6c677bc <Fix-#37715-accessor.py> (round 2/2):
[50.00%] ·· Benchmarking virtualenv-py3.8-Cython0.29.32-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pyarrow-scipy-sqlalchemy-tables-xlrd-xlsxwriter
[75.00%] ··· strings.Cat.time_cat ok
[75.00%] ··· ============ ====== ======== ========= ============
other_cols sep na_rep na_frac
------------ ------ -------- --------- ------------
0 None None 0.0 14.1±2ms
0 None None 0.001 17.3±0.9ms
0 None None 0.15 17.5±0.6ms
0 None - 0.0 14.6±4ms
0 None - 0.001 8.90±0.4ms
0 None - 0.15 13.2±2ms
0 , None 0.0 15.4±0.7ms
0 , None 0.001 15.2±0.9ms
0 , None 0.15 17.7±1ms
0 , - 0.0 15.0±0.6ms
0 , - 0.001 9.58±0.3ms
0 , - 0.15 12.7±0.6ms
3 None None 0.0 44.5±0.8ms
3 None None 0.001 50.2±2ms
3 None None 0.15 55.0±3ms
3 None - 0.0 44.5±0.7ms
3 None - 0.001 47.8±0.7ms
3 None - 0.15 73.3±4ms
3 , None 0.0 70.7±2ms
3 , None 0.001 67.4±3ms
3 , None 0.15 58.7±4ms
3 , - 0.0 65.7±1ms
3 , - 0.001 65.0±2ms
3 , - 0.15 85.4±6ms
============ ====== ======== ========= ============
[75.00%] · For pandas commit bc987e7 (round 2/2):
[75.00%] ·· Building for virtualenv-py3.8-Cython0.29.32-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pyarrow-scipy-sqlalchemy-tables-xlrd-xlsxwriter....
[75.00%] ·· Benchmarking virtualenv-py3.8-Cython0.29.32-jinja2-matplotlib-numba-numexpr-numpy-odfpy-openpyxl-pyarrow-scipy-sqlalchemy-tables-xlrd-xlsxwriter
[100.00%] ··· strings.Cat.time_cat ok
[100.00%] ··· ============ ====== ======== ========= =============
other_cols sep na_rep na_frac
------------ ------ -------- --------- -------------
0 None None 0.0 9.90±2ms
0 None None 0.001 9.58±1ms
0 None None 0.15 12.8±2ms
0 None - 0.0 8.07±0.6ms
0 None - 0.001 10.3±2ms
0 None - 0.15 11.7±0.7ms
0 , None 0.0 8.87±1ms
0 , None 0.001 8.74±0.08ms
0 , None 0.15 10.7±2ms
0 , - 0.0 7.54±0.5ms
0 , - 0.001 8.46±0.5ms
0 , - 0.15 10.7±0.7ms
3 None None 0.0 44.5±1ms
3 None None 0.001 47.2±1ms
3 None None 0.15 52.4±4ms
3 None - 0.0 43.1±3ms
3 None - 0.001 49.1±3ms
3 None - 0.15 71.6±6ms
3 , None 0.0 72.6±10ms
3 , None 0.001 71.7±4ms
3 , None 0.15 62.2±6ms
3 , - 0.0 68.2±1ms
3 , - 0.001 66.7±2ms
3 , - 0.15 81.5±5ms
============ ====== ======== ========= =============
before after ratio
[bc987e70] [6c677bc2]
<Fix-#37715-accessor.py>
-
7.54±0.5ms 15.0±0.6ms 1.99 strings.Cat.time_cat(0, ',', '-', 0.0) -
8.07±0.6ms 14.6±4ms 1.81 strings.Cat.time_cat(0, None, '-', 0.0) -
9.58±1ms 17.3±0.9ms 1.81 strings.Cat.time_cat(0, None, None, 0.001) -
8.87±1ms 15.4±0.7ms 1.74 strings.Cat.time_cat(0, ',', None, 0.0) -
8.74±0.08ms 15.2±0.9ms 1.74 strings.Cat.time_cat(0, ',', None, 0.001) -
10.7±2ms 17.7±1ms 1.65 strings.Cat.time_cat(0, ',', None, 0.15) -
9.90±2ms 14.1±2ms 1.42 strings.Cat.time_cat(0, None, None, 0.0) -
12.8±2ms 17.5±0.6ms 1.36 strings.Cat.time_cat(0, None, None, 0.15) -
10.7±0.7ms 12.7±0.6ms 1.19 strings.Cat.time_cat(0, ',', '-', 0.15)
SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED.
It seems a bad result, does it mean I need to change something?
There was a problem hiding this comment.
Awesome thanks for doing that! Yea looks like wrapping this in a Series slows things down - do you see any other way of getting the typing to work without the Series construction?
There was a problem hiding this comment.
Let me try some other ways
pandas/core/strings/accessor.py
Outdated
| @@ -562,15 +562,15 @@ def cat( | |||
| if sep is None: | |||
| sep = "" | |||
|
|
|||
| data: Series | |||
There was a problem hiding this comment.
can you try data: Series | np.ndarray?
There was a problem hiding this comment.
You can run mypy locally to check for errors
|
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
|
Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen. |
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.