BUG/PERF: Use EA in Index.get_value #24204

TomAugspurger · 2018-12-10T14:51:12Z

Avoid converting the ExtensionArray to an ndarray.

Before

pandas/tests/arrays/categorical/test_indexing.py F                                                                                                   [100%]

========================================================================= FAILURES =========================================================================
______________________________________________________________________ test_series_at ______________________________________________________________________

    def test_series_at():
        arr = NonCoercaibleCategorical(['a', 'b', 'c'])
        ser = Series(arr)
>       result = ser.at[0]

pandas/tests/arrays/categorical/test_indexing.py:158:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
pandas/core/indexing.py:2266: in __getitem__
    return self.obj._get_value(*key, takeable=self._takeable)
pandas/core/series.py:1078: in _get_value
    return self.index.get_value(self._values, label)
pandas/core/indexes/base.py:4303: in get_value
    s = com.values_from_object(series)
pandas/_libs/lib.pyx:82: in pandas._libs.lib.values_from_object
    obj = func()
pandas/core/arrays/categorical.py:1509: in get_values
    return np.array(self)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <[ValueError("I cannot be converted.") raised in repr()] NonCoercaibleCategorical object at 0x113d8b6a0>, dtype = None

    def __array__(self, dtype=None):
>       raise ValueError("I cannot be converted.")
E       ValueError: I cannot be converted.

Perf:

In [3]: a = pd.Series(pd.Categorical(np.random.choice(list(string.ascii_letters[:10]), 10_000)))

In [4]: %timeit a.at[0]
143 µs ± 4.86 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

In [3]: a = pd.Series(pd.Categorical(np.random.choice(list(string.ascii_letters[:10]), 10_000)))

In [4]: %timeit a.at[0]
11.1 µs ± 95.2 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

Split from #24024

Avoid converting the ExtensionArray to an ndarray. Before ``` pandas/tests/arrays/categorical/test_indexing.py F [100%] ========================================================================= FAILURES ========================================================================= ______________________________________________________________________ test_series_at ______________________________________________________________________ def test_series_at(): arr = NonCoercaibleCategorical(['a', 'b', 'c']) ser = Series(arr) > result = ser.at[0] pandas/tests/arrays/categorical/test_indexing.py:158: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ pandas/core/indexing.py:2266: in __getitem__ return self.obj._get_value(*key, takeable=self._takeable) pandas/core/series.py:1078: in _get_value return self.index.get_value(self._values, label) pandas/core/indexes/base.py:4303: in get_value s = com.values_from_object(series) pandas/_libs/lib.pyx:82: in pandas._libs.lib.values_from_object obj = func() pandas/core/arrays/categorical.py:1509: in get_values return np.array(self) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = <[ValueError("I cannot be converted.") raised in repr()] NonCoercaibleCategorical object at 0x113d8b6a0>, dtype = None def __array__(self, dtype=None): > raise ValueError("I cannot be converted.") E ValueError: I cannot be converted. ``` Perf: ``` In [3]: a = pd.Series(pd.Categorical(np.random.choice(list(string.ascii_letters[:10]), 10_000))) In [4]: %timeit a.at[0] 143 µs ± 4.86 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each) In [3]: a = pd.Series(pd.Categorical(np.random.choice(list(string.ascii_letters[:10]), 10_000))) In [4]: %timeit a.at[0] 11.1 µs ± 95.2 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each) ```

pep8speaks · 2018-12-10T14:51:21Z

Hello @TomAugspurger! Thanks for submitting the PR.

There are no PEP8 issues in the file pandas/core/indexes/base.py !
There are no PEP8 issues in the file pandas/tests/arrays/categorical/test_indexing.py !

TomAugspurger · 2018-12-10T14:53:15Z

pandas/tests/arrays/categorical/test_indexing.py

@@ -145,3 +145,15 @@ def test_mask_with_boolean_raises(index):

    with pytest.raises(ValueError, match='NA / NaN'):
        s[idx]
+
+
+class NonCoercaibleCategorical(Categorical):


I imagine this pattern would be generally useful to detect places where we accidentally convert an EA to an ndarray. What would be the best way to share this among tests? Would making a fixture that returned this be too weird? Monkeypatching?

could do this with a pytest monkeypatch (maybe just create an issue to follow up)

codecov · 2018-12-10T15:41:06Z

Codecov Report

Merging #24204 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #24204   +/-   ##
=======================================
  Coverage   92.21%   92.21%           
=======================================
  Files         162      162           
  Lines       51723    51723           
=======================================
  Hits        47694    47694           
  Misses       4029     4029

Flag	Coverage Δ
#multiple	`90.61% <100%> (ø)`	⬆️
#single	`43.03% <100%> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/indexes/base.py	`96.27% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5a4711...7a6a16d. Read the comment docs.

codecov · 2018-12-10T15:41:09Z

Codecov Report

Merging #24204 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #24204      +/-   ##
==========================================
+ Coverage   92.21%   92.21%   +<.01%     
==========================================
  Files         162      162              
  Lines       51723    51761      +38     
==========================================
+ Hits        47694    47731      +37     
- Misses       4029     4030       +1

Flag	Coverage Δ
#multiple	`90.61% <100%> (ø)`	⬆️
#single	`43% <100%> (-0.03%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/base.py	`96.27% <100%> (ø)`	⬆️
pandas/core/arrays/categorical.py	`95.3% <0%> (-0.1%)`	⬇️
pandas/core/generic.py	`96.65% <0%> (ø)`	⬆️
pandas/core/arrays/sparse.py	`92.08% <0%> (ø)`	⬆️
pandas/core/arrays/base.py	`97.41% <0%> (ø)`	⬆️
pandas/core/indexes/category.py	`97.9% <0%> (ø)`	⬆️
pandas/core/internals/blocks.py	`93.81% <0%> (+0.13%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5a4711...49c5967. Read the comment docs.

jreback

if you'd just document what you are doing (and fix spelling), looks good to me.

pandas/tests/arrays/categorical/test_indexing.py

jreback · 2018-12-10T15:46:17Z

pandas/tests/arrays/categorical/test_indexing.py

@@ -145,3 +145,15 @@ def test_mask_with_boolean_raises(index):

    with pytest.raises(ValueError, match='NA / NaN'):
        s[idx]
+
+
+class NonCoercaibleCategorical(Categorical):


could do this with a pytest monkeypatch (maybe just create an issue to follow up)

TomAugspurger · 2018-12-10T17:16:42Z

All green.

TomAugspurger · 2018-12-10T17:38:01Z

Opened #24207 for followup.

jreback · 2018-12-10T17:59:31Z

thanks!

TomAugspurger added Performance Memory or execution speed performance ExtensionArray Extending pandas with custom dtypes or arrays. labels Dec 10, 2018

TomAugspurger added this to the 0.24.0 milestone Dec 10, 2018

issue number

7a6a16d

TomAugspurger commented Dec 10, 2018

View reviewed changes

TomAugspurger mentioned this pull request Dec 10, 2018

REF: DatetimeLikeArray #24024

Merged

12 tasks

jreback approved these changes Dec 10, 2018

View reviewed changes

Use monkeypatch

49c5967

TomAugspurger mentioned this pull request Dec 10, 2018

Add tests asserting that EAs shouldn't be coerced to ndarray #24207

Open

jreback merged commit 91802fb into pandas-dev:master Dec 10, 2018

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

BUG/PERF: Use EA in Index.get_value (pandas-dev#24204)

43c4500

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

BUG/PERF: Use EA in Index.get_value (pandas-dev#24204)

2905462

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG/PERF: Use EA in Index.get_value #24204

BUG/PERF: Use EA in Index.get_value #24204

TomAugspurger commented Dec 10, 2018

pep8speaks commented Dec 10, 2018

TomAugspurger Dec 10, 2018

jreback Dec 10, 2018

codecov bot commented Dec 10, 2018

codecov bot commented Dec 10, 2018 •

edited

Loading

jreback left a comment

jreback Dec 10, 2018

TomAugspurger commented Dec 10, 2018

TomAugspurger commented Dec 10, 2018

jreback commented Dec 10, 2018

BUG/PERF: Use EA in Index.get_value #24204

BUG/PERF: Use EA in Index.get_value #24204

Conversation

TomAugspurger commented Dec 10, 2018

pep8speaks commented Dec 10, 2018

TomAugspurger Dec 10, 2018

Choose a reason for hiding this comment

jreback Dec 10, 2018

Choose a reason for hiding this comment

codecov bot commented Dec 10, 2018

Codecov Report

codecov bot commented Dec 10, 2018 • edited Loading

Codecov Report

jreback left a comment

Choose a reason for hiding this comment

jreback Dec 10, 2018

Choose a reason for hiding this comment

TomAugspurger commented Dec 10, 2018

TomAugspurger commented Dec 10, 2018

jreback commented Dec 10, 2018

codecov bot commented Dec 10, 2018 •

edited

Loading