Ensure Index._data is an ndarray #23628

TomAugspurger · 2018-11-11T21:14:22Z

Split from #23623, where it was
causing issues.

BUG: Ensure that Index._data is an ndarray Split from pandas-dev#23623, where it was causing issues with infer_dtype.

pep8speaks · 2018-11-11T21:14:26Z

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/indexes/test_base.py !
There are no PEP8 issues in the file pandas/tests/indexes/test_numeric.py !

pandas/core/indexes/base.py

jreback · 2018-11-11T22:57:05Z

lgtm. circle seems to be failing lots of PRs today.

jorisvandenbossche

Should we rather fix the places where _simple_new is called with an Index or Series ? Or are that many places (I suppose the bool code path) ?

TomAugspurger · 2018-11-12T11:52:59Z

I started fixing the callers, but gave up when I saw all the potential call sites. It's not just boolean index, the numeric ones hit this as well.

TomAugspurger · 2018-11-12T12:17:56Z

I didn't look into why the expected failures in
fa8934a changed.

codecov · 2018-11-12T12:52:53Z

Codecov Report

❗ No coverage uploaded for pull request base (master@6d031f2). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #23628   +/-   ##
=========================================
  Coverage          ?   92.24%           
=========================================
  Files             ?      161           
  Lines             ?    51341           
  Branches          ?        0           
=========================================
  Hits              ?    47362           
  Misses            ?     3979           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.64% <100%> (?)`
#single	`42.34% <100%> (?)`

Impacted Files	Coverage Δ
pandas/core/indexes/base.py	`96.46% <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 6d031f2...265ffcb. Read the comment docs.

TomAugspurger · 2018-11-12T14:08:44Z

All green.

pandas/tests/series/test_operators.py

[ci skip] ***NO CI***

jbrockmendel · 2018-11-12T21:43:32Z

Should we rather fix the places where _simple_new is called with an Index or Series ?

+1 for the "sooner side of eventually" pile

TomAugspurger · 2018-11-12T21:50:15Z

@jbrockmendel can you comment on the skips / expected values in
https://github.com/pandas-dev/pandas/pull/23628/files/fa8934a3082300c209b4dddc9eb7e34029b6493f#diff-078718e1fe56beae8c2e3be2405f4d19?

It seems to me like the expected values are incorrect. Consider op=ops.rand_. In that case, we're saying that

ops.rand_(series, index) -> Series

but it seems to me that ops.and_(index, series) -> Index, right?

edit: just saw your reply.

TomAugspurger · 2018-11-12T22:10:55Z

Based on my thinking in
#23628 (comment), I've moved the reverse ops to a different test in e4b21f6. Let me know if that seems completely wrong-headed.

TomAugspurger · 2018-11-13T16:41:28Z

Any objections to merging? (this is blocking IndexOpsMinx.array / to_numpy())

jreback

minor comment

jreback · 2018-11-14T02:25:10Z

pandas/core/indexes/base.py

@@ -522,6 +522,12 @@ def _simple_new(cls, values, name=None, dtype=None, **kwargs):
                    values = cls(values, name=name, dtype=dtype,
                                 **kwargs)._ndarray_values

+        if isinstance(values, (ABCSeries, cls)):


should use ABCIndexClass here? it’s acrually more clear

I can never remember whether ABCIndexClass or ABCIndex includes subclasses, so I was happy when I realized I could just use cls here :)

ABCIndexClass - it’s just more consistent in the code

jorisvandenbossche · 2018-11-14T10:33:54Z

@TomAugspurger just to be clear: regarding the behaviour of xor with Index / Series, did you decide that the current behaviour is correct? (as you added a test for it)?

TomAugspurger · 2018-11-14T12:10:14Z

To be clear, I added a test for `roxr(Series, Index)`. And if we're using the most sensible definition of roxr(Series, Index) == xor(Index, Series) then yes, the test should be correct. In that case the expected is `Index([])`, since they both contained True and False.

…

On Wed, Nov 14, 2018 at 4:34 AM Joris Van den Bossche < ***@***.***> wrote: @TomAugspurger <https://github.com/TomAugspurger> just to be clear: regarding the behaviour of xor with Index / Series, did you decide that the current behaviour is correct? (as you added a test for it)? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#23628 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQHIut2wEt79DaAYgZzvn3fB731C6ykks5uu_GXgaJpZM4YYsIq> .

jreback · 2018-11-14T12:56:38Z

pandas/tests/indexes/test_base.py

@@ -504,6 +504,12 @@ def test_constructor_cast(self):
        with pytest.raises(ValueError, match=msg):
            Index(["a", "b", "c"], dtype=float)

+    def test_constructor_unwraps_index(self):


you can make this more general by using the indices fixture here.

jreback · 2018-11-14T12:56:53Z

pandas/tests/indexes/test_numeric.py

@@ -628,6 +628,12 @@ def test_constructor_coercion_signed_to_unsigned(self, uint_dtype):
        with pytest.raises(OverflowError, match=msg):
            Index([-1], dtype=uint_dtype)



rather than specificying testing a single index, see my above comment.

TomAugspurger · 2018-11-14T21:16:41Z

parametrized over indices (skipping MultiIndex).

TomAugspurger · 2018-11-15T12:22:55Z

All green.

jorisvandenbossche · 2018-11-15T12:57:29Z

OK for the xor behaviour (We should maybe consider deprecating it, but that's a separate issue)

jorisvandenbossche · 2018-11-15T12:58:03Z

@TomAugspurger Thanks!

* upstream/master: BUG: to_html misses truncation indicators (...) when index=False (pandas-dev#22786) API/DEPR: replace "raise_conflict" with "errors" for df.update (pandas-dev#23657) BUG: Append DataFrame to Series with dateutil timezone (pandas-dev#23685) CLN/CI: Catch that stderr-warning! (pandas-dev#23706) ENH: Allow for join between two multi-index dataframe instances (pandas-dev#20356) Ensure Index._data is an ndarray (pandas-dev#23628) DOC: flake8-per-pr for windows users (pandas-dev#23707) DOC: Handle exceptions when computing contributors. (pandas-dev#23714) DOC: Validate space before colon docstring parameters pandas-dev#23483 (pandas-dev#23506) BUG-22984 Fix truncation of DataFrame representations (pandas-dev#22987)

BUG: Ensure that Index._data is an ndarray

1f23ebc

BUG: Ensure that Index._data is an ndarray Split from pandas-dev#23623, where it was causing issues with infer_dtype.

remove assert

a30bc02

jreback reviewed Nov 11, 2018

View reviewed changes

pandas/core/indexes/base.py Show resolved Hide resolved

jreback added Reshaping Concat, Merge/Join, Stack/Unstack, Explode Dtype Conversions Unexpected or buggy dtype conversions labels Nov 11, 2018

jreback added this to the 0.24.0 milestone Nov 11, 2018

jorisvandenbossche reviewed Nov 12, 2018

View reviewed changes

TomAugspurger added 2 commits November 12, 2018 05:55

Merge remote-tracking branch 'upstream/master' into index-ndarray-data

505970e

update errors

fa8934a

jorisvandenbossche reviewed Nov 12, 2018

View reviewed changes

pandas/tests/series/test_operators.py Outdated Show resolved Hide resolved

Add note

e903550

[ci skip] ***NO CI***

TST: Change rops tests

e4b21f6

jreback approved these changes Nov 14, 2018

View reviewed changes

jreback requested changes Nov 14, 2018

View reviewed changes

TomAugspurger added 2 commits November 14, 2018 15:08

Merge remote-tracking branch 'upstream/master' into index-ndarray-data

f72e57a

parametrize

265ffcb

jorisvandenbossche merged commit fb68731 into pandas-dev:master Nov 15, 2018

tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018

Ensure Index._data is an ndarray (pandas-dev#23628)

eb0a01e

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

Ensure Index._data is an ndarray (pandas-dev#23628)

53a4b34

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

Ensure Index._data is an ndarray (pandas-dev#23628)

f3b1073

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure Index._data is an ndarray #23628

Ensure Index._data is an ndarray #23628

TomAugspurger commented Nov 11, 2018

pep8speaks commented Nov 11, 2018

jreback commented Nov 11, 2018

jorisvandenbossche left a comment

TomAugspurger commented Nov 12, 2018

TomAugspurger commented Nov 12, 2018

codecov bot commented Nov 12, 2018 •

edited

Loading

TomAugspurger commented Nov 12, 2018

jbrockmendel commented Nov 12, 2018

TomAugspurger commented Nov 12, 2018 •

edited

Loading

TomAugspurger commented Nov 12, 2018

TomAugspurger commented Nov 13, 2018 •

edited

Loading

jreback left a comment

jreback Nov 14, 2018

TomAugspurger Nov 14, 2018

jreback Nov 14, 2018

jorisvandenbossche commented Nov 14, 2018

TomAugspurger commented Nov 14, 2018 via email

jreback Nov 14, 2018

jreback Nov 14, 2018

TomAugspurger commented Nov 14, 2018

TomAugspurger commented Nov 15, 2018

jorisvandenbossche commented Nov 15, 2018

jorisvandenbossche commented Nov 15, 2018

		@@ -628,6 +628,12 @@ def test_constructor_coercion_signed_to_unsigned(self, uint_dtype):
		with pytest.raises(OverflowError, match=msg):
		Index([-1], dtype=uint_dtype)

Ensure Index._data is an ndarray #23628

Ensure Index._data is an ndarray #23628

Conversation

TomAugspurger commented Nov 11, 2018

pep8speaks commented Nov 11, 2018

jreback commented Nov 11, 2018

jorisvandenbossche left a comment

Choose a reason for hiding this comment

TomAugspurger commented Nov 12, 2018

TomAugspurger commented Nov 12, 2018

codecov bot commented Nov 12, 2018 • edited Loading

Codecov Report

TomAugspurger commented Nov 12, 2018

jbrockmendel commented Nov 12, 2018

TomAugspurger commented Nov 12, 2018 • edited Loading

TomAugspurger commented Nov 12, 2018

TomAugspurger commented Nov 13, 2018 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

jreback Nov 14, 2018

Choose a reason for hiding this comment

TomAugspurger Nov 14, 2018

Choose a reason for hiding this comment

jreback Nov 14, 2018

Choose a reason for hiding this comment

jorisvandenbossche commented Nov 14, 2018

TomAugspurger commented Nov 14, 2018 via email

jreback Nov 14, 2018

Choose a reason for hiding this comment

jreback Nov 14, 2018

Choose a reason for hiding this comment

TomAugspurger commented Nov 14, 2018

TomAugspurger commented Nov 15, 2018

jorisvandenbossche commented Nov 15, 2018

jorisvandenbossche commented Nov 15, 2018

codecov bot commented Nov 12, 2018 •

edited

Loading

TomAugspurger commented Nov 12, 2018 •

edited

Loading

TomAugspurger commented Nov 13, 2018 •

edited

Loading