DEPR: remove Panel-specific parts of core.indexing #25567

simonjayhawkins · 2019-03-06T11:11:38Z

follow-on from #25550

codecov · 2019-03-06T11:46:45Z

Codecov Report

Merging #25567 into master will increase coverage by 0.05%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #25567      +/-   ##
==========================================
+ Coverage   91.26%   91.31%   +0.05%     
==========================================
  Files         173      173              
  Lines       52966    52928      -38     
==========================================
- Hits        48337    48329       -8     
+ Misses       4629     4599      -30

Flag	Coverage Δ
#multiple	`89.88% <100%> (+0.04%)`	⬆️
#single	`41.73% <50%> (+0.02%)`	⬆️

Impacted Files	Coverage Δ
pandas/core/indexing.py	`93.46% <100%> (+2.58%)`	⬆️

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 3e652ac...c47d36f. Read the comment docs.

codecov · 2019-03-06T11:46:46Z

Codecov Report

Merging #25567 into master will increase coverage by 0.06%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #25567      +/-   ##
==========================================
+ Coverage   91.77%   91.83%   +0.06%     
==========================================
  Files         174      174              
  Lines       50681    50643      -38     
==========================================
- Hits        46513    46510       -3     
+ Misses       4168     4133      -35

Flag	Coverage Δ
#multiple	`90.37% <100%> (+0.06%)`	⬆️
#single	`41.69% <50%> (-0.06%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexing.py	`93.48% <100%> (+2.95%)`	⬆️
pandas/io/gbq.py	`78.94% <0%> (-10.53%)`	⬇️
pandas/core/frame.py	`97% <0%> (-0.12%)`	⬇️
pandas/core/generic.py	`93.62% <0%> (+0.04%)`	⬆️
pandas/core/arrays/datetimelike.py	`97.88% <0%> (+0.17%)`	⬆️
pandas/core/arrays/timedeltas.py	`88.8% <0%> (+0.19%)`	⬆️
pandas/util/testing.py	`90.81% <0%> (+0.21%)`	⬆️

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 8154efb...eaf161b. Read the comment docs.

jreback · 2019-03-06T12:41:44Z

pandas/core/indexing.py

@@ -693,7 +686,6 @@ def ravel(i):
            sum_aligners = sum(aligners)
            single_aligner = sum_aligners == 1
            is_frame = self.obj.ndim == 2
-            is_panel = self.obj.ndim >= 3


I think we are going to need a new error if self.obj.ndim.ndim > 2 (e.g. a numpy array); need to check both for setting & getting (it *might be already there but not sure about tests)

jreback · 2019-03-10T16:53:39Z

can you merge master; also created #25632 for things like this

simonjayhawkins · 2019-03-13T13:32:17Z

merged master to remove overlap with #25675

jreback · 2019-03-13T15:29:49Z

pandas/_libs/indexing.pyx

@@ -20,4 +20,5 @@ cdef class _NDFrameIndexerBase:
        ndim = self._ndim
        if ndim is None:
            ndim = self._ndim = self.obj.ndim
+            assert not ndim > 2, 'Internal Error: NDFrameIndexer ndim > 2'


is this ever raised in user code? what happens when a 3-d indexer is passed?

can you add some sort of test for this (also this should raise a ValueError if its user code)

is this ever raised in user code? what happens when a 3-d indexer is passed?

added some tests for 3d indexing (integer ndarray only at the moment). not sure if this was what was required. tests is messy with mixture of some getting and setting not raising and those that raise produce a range of error messages.

can you add some sort of test for this (also this should raise a ValueError if its user code)

will look into this next.

also this should raise a ValueError if its user code

since NDFrame is not part of the API, i'm not sure how this can be raised from user code. (other than Panel)

i've added a test for NDFrame initialized with 3d array, but it only raises this exception with .ix

jreback · 2019-03-22T11:57:21Z

pandas/tests/indexing/test_indexing.py

+        lambda i: DataFrame(
+            np.random.randn(len(i), len(i)), index=i, columns=i)
+    ], ids=['Series', 'DataFrame'])
+    @pytest.mark.parametrize('idxr, idxr_id', [


might be worth making these a fixture at somepoint (the iteration over indexers)

the iteration over indexers

yeah. i appreciate we have duplication of parameterization at the moment, and fixtures may be of use. didn't carry on down that route at the moment as i wasn't sure if these tests were what was required.

separately for iteration over the indexes:

xref #25748 (comment). There is a indices fixture in pandas\tests\indexes\conftest.py. Maybe worth promoting that up a level so that it can also be used in pandas\tests\indexing

jreback · 2019-03-22T11:57:59Z

pandas/tests/indexing/test_indexing.py

+               "unhashable type: 'numpy.ndarray'"  # TypeError
+               )
+
+        if (isinstance(obj, Series) and idxr_id == 'getitem'


ugg this is why getitem is so complicated.

jreback · 2019-04-05T00:49:50Z

can you merge master. code change looks good, can you simplify the tests at all? maybe split them up a bit?

jreback · 2019-04-20T16:59:07Z

can you merge master

jreback · 2019-05-12T21:18:22Z

@simonjayhawkins can you merge master and update

WillAyd · 2019-05-21T01:01:34Z

@simonjayhawkins can you merge master and fix up again? I think this should take a pretty big chunk out of what remains for Panel

simonjayhawkins · 2019-05-30T12:23:38Z

@simonjayhawkins can you merge master and fix up again? I think this should take a pretty big chunk out of what remains for Panel

i've fixed up test_ndframe_indexing_raises so that the tests pass.

This now is basically a regression test. it does check the new error message added within this PR for .ix only.

For the other indexers, other errors are raised before it gets to the deferred ndim lookup in indexing.pyx

can you simplify the tests at all? maybe split them up a bit?

the tests are already split by getting and setting, but as the parametrisation is also across indexes, pandas object types, and indexer that results in 88 test cases in each of the two tests.

the reason parametrisation was used is to check for consistency of error messages raised.

The error messages are not consistent at the moment and some permutations don't raise (they probably should as the output is some cases is non nonsensical). will need to raise some issues here to make the behavior more consistent.

This results in tests that are visually messy.

So again, test_getitem_ndarray_3d and test_setitem_ndarray_3d should be viewed as regression tests and not expected behavior.

if they should be split again, probably best to split on indexer?

jreback · 2019-05-30T12:28:39Z

ok looks better

ping on green

simonjayhawkins · 2019-05-30T13:11:55Z

@jreback pinging on green.

jreback · 2019-05-30T13:15:26Z

thanks!

DEPR: remove Panel-specific parts of core.indexing

c47d36f

jreback added Indexing Related to indexing on series/frames, not to indexes themselves Panel labels Mar 6, 2019

jreback added this to the 0.25.0 milestone Mar 6, 2019

jreback requested changes Mar 6, 2019

View reviewed changes

simonjayhawkins added 3 commits March 8, 2019 10:58

Merge remote-tracking branch 'upstream/master' into panel-indexing

6e485bd

add new error if ndim > 2 and remove failing test

348c595

minor change

451a967

simonjayhawkins added 2 commits March 10, 2019 22:19

Merge remote-tracking branch 'upstream/master' into panel-indexing

63495a6

Merge remote-tracking branch 'upstream/master' into panel-indexing

cc22e5f

jreback reviewed Mar 13, 2019

View reviewed changes

simonjayhawkins added 3 commits March 19, 2019 10:37

Merge remote-tracking branch 'upstream/master' into panel-indexing

bc505bc

add 3-d indexer setting and getting tests

f39a75f

Merge remote-tracking branch 'upstream/master' into panel-indexing

c1c200e

jreback reviewed Mar 22, 2019

View reviewed changes

simonjayhawkins added 5 commits March 28, 2019 20:36

resolve merge conflict

fac9442

Merge remote-tracking branch 'upstream/master' into panel-indexing

be19f78

change exception raised and add test

26f2ac2

fix linting errors

1de10b5

Merge remote-tracking branch 'upstream/master' into panel-indexing

8d2ab43

WillAyd mentioned this pull request May 10, 2019

Remove Panel References from Tests #26332

Merged

Merge remote-tracking branch 'upstream/master' into panel-indexing

496ab7d

simonjayhawkins added 2 commits May 30, 2019 12:27

Merge remote-tracking branch 'upstream/master' into panel-indexing

29cde25

fix-up test_ndframe_indexing_raises

eaf161b

jreback approved these changes May 30, 2019

View reviewed changes

jreback merged commit a60d1bd into pandas-dev:master May 30, 2019

simonjayhawkins deleted the panel-indexing branch May 30, 2019 16:48

simonjayhawkins mentioned this pull request Jun 8, 2019

CLN: remove Panel-specific parts of core.generic #26738

Merged

arw2019 mentioned this pull request Sep 24, 2020

BUG: set_index with passing key of first level of MI produces invalid result #24683

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DEPR: remove Panel-specific parts of core.indexing #25567

DEPR: remove Panel-specific parts of core.indexing #25567

simonjayhawkins commented Mar 6, 2019

codecov bot commented Mar 6, 2019

codecov bot commented Mar 6, 2019 •

edited

Loading

jreback Mar 6, 2019

jreback commented Mar 10, 2019

simonjayhawkins commented Mar 13, 2019

jreback Mar 13, 2019

jreback Mar 20, 2019

simonjayhawkins Mar 22, 2019

simonjayhawkins Mar 30, 2019

jreback Mar 22, 2019

simonjayhawkins Mar 22, 2019

jreback Mar 22, 2019

jreback commented Apr 5, 2019

jreback commented Apr 20, 2019

jreback commented May 12, 2019

WillAyd commented May 21, 2019

simonjayhawkins commented May 30, 2019

jreback commented May 30, 2019

simonjayhawkins commented May 30, 2019

jreback commented May 30, 2019

DEPR: remove Panel-specific parts of core.indexing #25567

DEPR: remove Panel-specific parts of core.indexing #25567

Conversation

simonjayhawkins commented Mar 6, 2019

codecov bot commented Mar 6, 2019

Codecov Report

codecov bot commented Mar 6, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

jreback commented Mar 10, 2019

simonjayhawkins commented Mar 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Apr 5, 2019

jreback commented Apr 20, 2019

jreback commented May 12, 2019

WillAyd commented May 21, 2019

simonjayhawkins commented May 30, 2019

jreback commented May 30, 2019

simonjayhawkins commented May 30, 2019

jreback commented May 30, 2019

codecov bot commented Mar 6, 2019 •

edited

Loading