BUG: names on union and intersection for Index were inconsistent (GH9943 GH9862) #19849

Dr-Irv · 2018-02-22T22:05:05Z

tests added / passed
- Added tests to check names on unions when Index has different values
- Fixed test for ranges with unions
- Added tests for all index types for corner cases for union, difference
- Added tests so that index.intersection(empty_index)==index.difference(index)
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Also fixes bug in Index.difference when computing index.difference(index)

gfyoung · 2018-02-23T09:15:38Z

@Dr-Irv @jreback : Are you sure we can close #9862 with this PR? Seems to be the case that it can given some of the discussion that took place. However, just want to double-check.

gfyoung · 2018-02-23T09:16:13Z

pandas/core/indexes/base.py

+            name = self.name
+        return name
+
+    def _get_intersection_name(self, other):


crayxt · 2018-02-23T10:36:14Z

doc/source/whatsnew/v0.23.0.txt

@@ -796,6 +796,7 @@ Indexing
 - Bug in :func:`IntervalIndex.symmetric_difference` where the symmetric difference with a non-``IntervalIndex`` did not raise (:issue:`18475`)
 - Bug in :class:`IntervalIndex` where set operations that returned an empty ``IntervalIndex`` had the wrong dtype (:issue:`19101`)
 - Bug in :meth:`DataFrame.drop_duplicates` where no ``KeyError`` is raised when passing in columns that don't exist on the ``DataFrame`` (issue:`19726`)
+- Bug in :func:`Index.union` where resulting names were not computed correrctly (:issue:`9943`, :issue:`9862`)


correrctly -> correctly

Dr-Irv · 2018-02-23T15:45:05Z

@gfyoung Re your question above about closing #9862, that was my sense based on the comments and my analysis of the open checkmarks at the top.

BTW, I'm working on those bugs identified in testing. Seems like I didn't have 'numexpr' installed on my local machine so those errors weren't picked up when I ran pytest locally.

codecov · 2018-02-24T15:04:29Z

Codecov Report

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

@@            Coverage Diff            @@
##             master   #19849   +/-   ##
=========================================
  Coverage          ?   92.24%           
=========================================
  Files             ?      161           
  Lines             ?    51193           
  Branches          ?        0           
=========================================
  Hits              ?    47225           
  Misses            ?     3968           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.63% <96.15%> (?)`
#single	`42.29% <59.61%> (?)`

Impacted Files	Coverage Δ
pandas/core/indexes/category.py	`97.87% <100%> (ø)`
pandas/core/indexes/period.py	`92.17% <100%> (ø)`
pandas/core/indexes/interval.py	`94.66% <100%> (ø)`
pandas/core/indexes/numeric.py	`97.32% <100%> (ø)`
pandas/core/indexes/base.py	`96.45% <100%> (ø)`
pandas/core/indexes/range.py	`97.32% <100%> (ø)`
pandas/core/indexes/timedeltas.py	`89.88% <83.33%> (ø)`
pandas/core/indexes/datetimes.py	`96.13% <88.88%> (ø)`

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 6a5c34c...fa7311a. Read the comment docs.

jreback

so rather than add all of these tests here, they should go in pandas/tests/indexes/test_base.py which generically tests all index types. further should move the existing setops tests there as well (could be a pre- or post- PR)

jreback · 2018-02-24T16:24:11Z

pandas/core/indexes/base.py

-                name = None
-            if self.name != name:
-                return self._shallow_copy(name=name)
+        return self.name if self.name == other.name else None


this logic already exists in pandas.core.ops.get_op_result_name I would just use this as a function and not as a method

A function added just a few days ago! Will make that change.

jreback · 2018-02-24T16:27:42Z

pandas/core/indexes/base.py

+        return a new object if we are resetting the name
+        """
+        name = self._get_setop_name(other)
+        if self.name != name:


I think might be able to dispense with this entirely, and just always do

from pandas.core.ops import get_op_result_name .... return self._shallow_copy(name=get_op_result_name(self, other))

do this in _wrap_setup_result

So if I eliminate self.get_setop_name_object and use _wrap_setop_result, other tests fail, because those tests assume the corner cases return the same object.

jreback · 2018-02-24T16:28:24Z

pandas/tests/indexes/test_base.py

@@ -729,7 +729,15 @@ def test_union(self):
        union = Index([]).union(first)
        assert union is first

-        # preserve names


make new tests

jreback · 2018-02-24T16:28:29Z

pandas/tests/indexes/test_base.py

@@ -751,37 +759,66 @@ def test_union(self):
        first = Index(list('ab'))
        second = Index(list('ab'), name='B')
        union = first.union(second)
-        expected = Index(list('ab'), name='B')


make new tests

jreback · 2018-02-24T16:28:34Z

pandas/tests/indexes/test_base.py

        tm.assert_index_equal(union, expected)

        first = Index([], name='A')
        second = Index(list('ab'))
        union = first.union(second)
-        expected = Index(list('ab'), name='A')
+        expected = Index(list('ab'), name=None)


@jreback Not sure what you mean by "make new tests". The issue with the existing tests is that they were incorrect. So I had to fix them. For example,

first = Index([], name='A') second = Index(list('ab')) union = first.union(second) expected = Index(list('ab'), name='A')

is incorrect, as if one index has a name, and the other has no name, the result should be None, based on your comment #9943 (comment)

I meant that these tests are getting very long, so split up a bit.

We also need more general testing, all of these tests should be using a generically created index (all of this machinery already exists ia .create_index())

I've split the test_union test, and now have the corner cases tested for all indexes

…943 GH9862)

jreback · 2018-02-27T11:07:15Z

doc/source/whatsnew/v0.23.0.txt

@@ -837,6 +837,7 @@ Indexing
 - Bug in :func:`IntervalIndex.symmetric_difference` where the symmetric difference with a non-``IntervalIndex`` did not raise (:issue:`18475`)
 - Bug in :class:`IntervalIndex` where set operations that returned an empty ``IntervalIndex`` had the wrong dtype (:issue:`19101`)
 - Bug in :meth:`DataFrame.drop_duplicates` where no ``KeyError`` is raised when passing in columns that don't exist on the ``DataFrame`` (issue:`19726`)
+- Bug in :func:`Index.union` where resulting names were not computed correctly for certain cases (:issue:`9943`, :issue:`9862`)


is the only user facing change? e.g. intersection as well? 'resulting names' is not vey clear, maybe 'the name of the Index of the union'

This is now changed as you suggested. intersection was computing names correctly despite the original example reported in #9943.

jreback · 2018-02-27T11:08:56Z

pandas/core/indexes/base.py

-        Given 2 indexes, give a consensus name meaning
-        we take the not None one, or None if the names differ.
-        Return a new object if we are resetting the name
+        Given 2 indexes, give a setop name and object, meaning


instead why can't we just use _wrap_setop_result? instead of having 2 nearly identical funtions

@jreback I think this is needed (and I'll rename it and document it accordingly) in the case you are computing i1.union(i2), where i1.equals(i2)==True. In this case, you don't want to use _shallow_copy() because you want to return i1, unless the names of i1 and i2 are different. There are some tests for joins where if you are doing a join and the index is the same, then you can just return the index, rather than returning a brand new index, which is what _shallow_copy() would do.

I'll also check the other Index subclasses to see if they are doing this right. For example, for RangeIndex, it is not handling the names correctly.

hmm, I think we do this elsewhere as well (e.g. when you reindex to yourself). can you add docmentation about this to the setops themselves. @TomAugspurger @jorisvandenbossche what do you think about this?

I don't have a strong preference, but it'd be good to settle on consistent behavior between union and intersection (I think). What's the proposal here?

named & same name -> name

named & other name -> None?

named & unnamed -> name?

unnamed & named -> name?

unamed & unnamed -> None

@TomAugspurger not what I was talking about, the current logic is correct (and shouldn't change), what you have is right. This was more about whether if we have identical objects we should shallow copy or not.

My bad.

I don't have a strong preference, but I would tend towards always shallow copying? I hope people aren't relying on (idx | idx) is idx being true.

@TomAugspurger With respect to your naming, that is not what will happen. In the cases of named and unnamed, the result will be None. That's because if you do

j1 = pd.Index([1,2], name='j1') j2 = pd.Index([], name='j2') j3 = pd.Index([], name='j3')

then j1.union(j2).union(j3) returns Int64Index([1, 2], dtype='int64', name='j3'), while just changing the order to j3.union(j1).union(j2) returns Int64Index([1, 2], dtype='int64', name='j2'). So the behavior will be to only preserve the names if they are equal, and the result of these two union examples will have None as the name.

@jreback I can change to shallow copying, but I'll have to modify some other tests that are depending on the result not being a shallow copy.

yeah I think its better to _shallow_copy these, yes I understand some tests may change because of this (IOW they will no longer be is, just .equals)

jreback · 2018-02-27T11:10:16Z

pandas/core/indexes/timedeltas.py

-    def _wrap_union_result(self, other, result):
-        name = self.name if self.name == other.name else None
+    def _wrap_setop_result(self, other, result):
+        name = get_op_result_name(self, other)


you can remove this one

jreback · 2018-02-27T11:11:23Z

pandas/core/indexes/base.py

-        name = self.name if self.name == other.name else None
-        return self.__class__(result, name=name)
+    def _wrap_setop_result(self, other, result):
+        return self.__class__(result, name=get_op_result_name(self, other))


use self._shallow_copy this propagates meta-data, allows you to remove some sublcass implementations of this

@jreback I've done some testing with self.shallow_copy() and it broke things with respect to CategoricalIndex, and then the discussion in #10186 becomes relevant, as we'd need to decide what to do when taking the union of two categorical indexes that have different categories. So I'm going to leave this implementation as it was.

jreback · 2018-02-27T11:14:57Z

pandas/tests/indexes/test_base.py

        tm.assert_index_equal(union, expected)

        first = Index([], name='A')
        second = Index(list('ab'))
        union = first.union(second)
-        expected = Index(list('ab'), name='A')
+        expected = Index(list('ab'), name=None)


I meant that these tests are getting very long, so split up a bit.

We also need more general testing, all of these tests should be using a generically created index (all of this machinery already exists ia .create_index())

…riety of index types.

jreback · 2018-03-01T23:20:34Z

doc/source/whatsnew/v0.23.0.txt

@@ -837,6 +837,8 @@ Indexing
 - Bug in :func:`IntervalIndex.symmetric_difference` where the symmetric difference with a non-``IntervalIndex`` did not raise (:issue:`18475`)
 - Bug in :class:`IntervalIndex` where set operations that returned an empty ``IntervalIndex`` had the wrong dtype (:issue:`19101`)
 - Bug in :meth:`DataFrame.drop_duplicates` where no ``KeyError`` is raised when passing in columns that don't exist on the ``DataFrame`` (issue:`19726`)
+- Bug in :func:`Index.union` and :func:`Index.intersection` where name of the `Index` of the result was not computed correctly for certain cases (:issue:`9943`, :issue:`9862`)
+- Bug in :func:`Index.difference` when taking difference of set and itself as type was not preserved.


add this PR number is the issue, e.g. (:issue:`19849`)

Should "set" be "index"?

Made both of those changes.

jreback · 2018-03-01T23:22:34Z

pandas/core/indexes/base.py

@@ -2441,6 +2454,13 @@ def intersection(self, other):
            taken.name = None
        return taken

+    def _create_empty_index(self, name):


don't create new API, this is just self._shallow_copy([], name=name)

@jreback So the issue here is that self._shallow_copy([], name=name) doesn't work correctly for RangeIndex. I can modify RangeIndex._self_shallow_copy to allow [] as an argument. For an index of strings (objects) or an index of dtype int64, self._shallow_copy([], name=name) doesn't preserve the dtype. And I haven't even looked at all the types yet. So now the changes get more involved to handle this corner case of creating an empty index, which is why I created the create_empty_index. The idea here is to have an empty index that preserves all the other attributes (dtype, categories, range step, etc.) of the class of self. self._shallow_copy([], name=name) does not do that without a lot of modifications. If you want me to do those modifications, I can look into that, but they might be pretty substantial.

jreback · 2018-03-01T23:22:44Z

pandas/core/indexes/category.py

@@ -716,6 +723,13 @@ def insert(self, loc, item):
        codes = np.concatenate((codes[:loc], code, codes[loc:]))
        return self._create_from_codes(codes)

+    def _create_empty_index(self, name):


jreback

pls try for minimal changes.

jreback · 2018-03-01T23:23:38Z

pandas/core/indexes/base.py

        return self

+    def _union_corner_case(self, other):


is there a reason you are creatijng this as a separate method? why not inline it

+1 for inlining it.

@jreback @TomAugspurger The subclasses for DateTimeIndex, RangeIndex and TimeDeltaIndex need to call this method from their union implementations. Should I inline it there too and have repeated code?

ok this PR looks a lot better. I still don't like this hlper function. can we refactor to not use it

jreback · 2018-03-01T23:24:12Z

pandas/core/indexes/multi.py

@@ -2733,7 +2733,7 @@ def intersection(self, other):
        other_tuples = other._ndarray_values
        uniq_tuples = sorted(set(self_tuples) & set(other_tuples))
        if len(uniq_tuples) == 0:
-            return MultiIndex(levels=[[]] * self.nlevels,
+            return MultiIndex(levels=self.levels,


why are you changing this?

@jreback IMHO, if the resulting intersection is empty, the levels should be preserved. Here's a simple example:

mi = pd.MultiIndex.from_product([['a','b','c'],[1,2]], names=['ll','nn']) print(mi.drop(mi)) print(mi.intersection([]))

This produces (with v0.22.0):

MultiIndex(levels=[['a', 'b', 'c'], [1, 2]], labels=[[], []], names=['ll', 'nn']) MultiIndex(levels=[[], []], labels=[[], []], names=['ll', 'nn'])

Doing this change as I documented it makes the set algebra consistent.

TomAugspurger

I'm finding this all a bit hard to follow as well, since you have to hack around other issues in pandas.

FWIW, fixing #10186 is on my medium-term TODO list, but probably not for 0.23.0. If that was solved, how much would the changes here be simplified? What other inconsistencies would have to be worked around?

TomAugspurger · 2018-03-02T14:37:16Z

doc/source/whatsnew/v0.23.0.txt

@@ -837,6 +837,8 @@ Indexing
 - Bug in :func:`IntervalIndex.symmetric_difference` where the symmetric difference with a non-``IntervalIndex`` did not raise (:issue:`18475`)
 - Bug in :class:`IntervalIndex` where set operations that returned an empty ``IntervalIndex`` had the wrong dtype (:issue:`19101`)
 - Bug in :meth:`DataFrame.drop_duplicates` where no ``KeyError`` is raised when passing in columns that don't exist on the ``DataFrame`` (issue:`19726`)
+- Bug in :func:`Index.union` and :func:`Index.intersection` where name of the `Index` of the result was not computed correctly for certain cases (:issue:`9943`, :issue:`9862`)


double-tick Index.

TomAugspurger · 2018-03-02T14:41:16Z

pandas/core/indexes/base.py

        return self

+    def _union_corner_case(self, other):


+1 for inlining it.

TomAugspurger · 2018-03-02T14:44:50Z

doc/source/whatsnew/v0.23.0.txt

@@ -837,6 +837,8 @@ Indexing
 - Bug in :func:`IntervalIndex.symmetric_difference` where the symmetric difference with a non-``IntervalIndex`` did not raise (:issue:`18475`)
 - Bug in :class:`IntervalIndex` where set operations that returned an empty ``IntervalIndex`` had the wrong dtype (:issue:`19101`)
 - Bug in :meth:`DataFrame.drop_duplicates` where no ``KeyError`` is raised when passing in columns that don't exist on the ``DataFrame`` (issue:`19726`)
+- Bug in :func:`Index.union` and :func:`Index.intersection` where name of the `Index` of the result was not computed correctly for certain cases (:issue:`9943`, :issue:`9862`)
+- Bug in :func:`Index.difference` when taking difference of set and itself as type was not preserved.


Should "set" be "index"?

Dr-Irv · 2018-03-02T16:00:16Z

@TomAugspurger With respect to your question about #10186, the issue comes down to whether we use self._shallow_copy() to return the result of set operations. When I tried using self._shallow_copy() in Index._wrap_setop_result(), and looked at what was happening, then CategoricalIndex was the problem. So it was easier to leave the existing implementation of Index._wrap_setop_result. If you were to handle #10186, then I could experiment more with changing the implementation of Index._wrap_setop_result.

jreback · 2018-03-07T14:35:21Z

@Dr-Irv as I said above. this needs to be greatly downscoped. You are changing way too many things and not using the current code conventions. So pls break this in to separate PR's.

Dr-Irv · 2018-03-07T16:32:30Z

@jreback wrote

as I said above. this needs to be greatly downscoped. You are changing way too many things and not using the current code conventions. So pls break this in to separate PR's.

My latest commit (fb3a9db) removes the changes relative to the bug in Index.difference, and I created a separate Issue for that (#20040), so we can discuss there the implementation choices.

jreback · 2018-03-07T20:29:59Z

pandas/tests/indexes/test_base.py

@@ -707,6 +707,98 @@ def test_intersect_str_dates(self):

        assert len(res) == 0

+    def create_empty_index(self, id):


what is this for?

I need a way to have an empty index that is independent of the individual Index class that is tested in test_corner_union. Alternatively, I could just call id.drop(id) each time I need to create an empty index for the test. IMHO (and I can be convinced otherwise), having the calls to _create_empty_index in the test makes it clear that an empty index is being used in the test.

jreback · 2018-03-07T20:30:07Z

pandas/tests/indexes/test_base.py

+        # GH 9943 9862
+        # Test unions with various name combinations
+        # Do not test MultiIndex
+


pls use parameterize

jreback · 2018-03-07T20:30:50Z

pandas/core/indexes/base.py

        return self

+    def _union_corner_case(self, other):


ok this PR looks a lot better. I still don't like this hlper function. can we refactor to not use it

Dr-Irv · 2018-03-07T20:42:54Z

@jreback with respect to the helper function Index._union_corner_case, the function is called from the implementations of union in DateTimeIndex, RangeIndex and TimeDeltaIndex. I can remove it by just repeating the code in those implementations. If you want the repeated code, let me know.

jreback · 2018-03-07T22:25:15Z

pandas/core/indexes/category.py

@@ -300,6 +301,12 @@ def itemsize(self):
        # Size of the items in categories, not codes.
        return self.values.itemsize

+    def _wrap_setop_result(self, other, result):
+        name = get_op_result_name(self, other)


use _shallow_copy here, we never call _simple_new directly. (another reason why the super method should use _shallow_copy, but that's for new PR)

can you move this method to Index.base? does it break anything?

jreback · 2018-03-07T22:25:55Z

pandas/core/indexes/datetimes.py

@@ -1136,6 +1137,11 @@ def union(self, other):
        y : Index or DatetimeIndex
        """
        self._assert_can_do_setop(other)
+


can you just call the super .union() here (after any conversion isneeded)? then you can inline the union_corner bizness

jreback · 2018-03-07T22:26:22Z

pandas/core/indexes/range.py

-            return self
-        if len(self) == 0:
-            return other
+        is_corner_case, corner_result = self._union_corner_case(other)


jreback · 2018-03-07T22:26:36Z

pandas/core/indexes/timedeltas.py

@@ -545,6 +546,11 @@ def union(self, other):
        y : Index or TimedeltaIndex
        """
        self._assert_can_do_setop(other)
+
+        is_corner_case, corner_result = self._union_corner_case(other)


jreback · 2018-03-07T22:27:43Z

pandas/tests/indexes/test_base.py

+
+        skip_index_keys = ['tuples', 'repeats']
+        for key, id in self.indices.items():
+            if key not in skip_index_keys:


use continue here

jreback · 2018-03-07T22:29:27Z

pandas/tests/indexes/test_base.py

+            ('copy', 'A', 'empty', 'B', None),
+            ('copy', 'A', 'empty', None, None),
+            ('copy', None, 'copy', 'B', None),
+            ('copy', None, 'copy', None, None),


do the empties in a separate test from the copies, ok on parameterize the names though (on both tests)

jbrockmendel · 2018-07-11T04:12:44Z

pandas/core/indexes/base.py

-        name = self.name if self.name == other.name else None
-        return self.__class__(result, name=name)
+    def _wrap_setop_result(self, other, result):
+        return self._constructor(result, name=get_op_result_name(self, other))


Would shallowcopy work here? Constructor isn’t used very often in the indexes.

@jbrockmendel repeating (part of) reply from a few months ago:

the issue comes down to whether we use self._shallow_copy() to return the result of set operations. When I tried using self._shallow_copy() in Index._wrap_setop_result(), and looked at what was happening, then CategoricalIndex was the problem. So it was easier to leave the existing implementation of Index._wrap_setop_result. If we were to handle #10186, then I could experiment more with changing the implementation of Index._wrap_setop_result.

jbrockmendel · 2018-07-11T04:13:30Z

This looks like a nice cleanup, thanks @Dr-Irv.

jreback · 2018-11-01T01:40:57Z

@Dr-Irv this got a bit lost, can you rebase

pep8speaks · 2018-11-01T16:31:54Z

Hello @Dr-Irv! Thanks for updating the PR.

There are no PEP8 issues in the file pandas/core/indexes/base.py !
There are no PEP8 issues in the file pandas/core/indexes/category.py !
There are no PEP8 issues in the file pandas/core/indexes/datetimes.py !
There are no PEP8 issues in the file pandas/core/indexes/interval.py !
There are no PEP8 issues in the file pandas/core/indexes/numeric.py !
There are no PEP8 issues in the file pandas/core/indexes/period.py !
There are no PEP8 issues in the file pandas/core/indexes/range.py !
There are no PEP8 issues in the file pandas/core/indexes/timedeltas.py !
There are no PEP8 issues in the file pandas/tests/indexes/common.py !
There are no PEP8 issues in the file pandas/tests/indexes/conftest.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/reshape/test_concat.py !

Dr-Irv · 2018-11-01T18:00:20Z

@jreback This is ready. The failure was due to a timeout on travis. If you want me to do something about that, let me know.

gfyoung · 2018-11-01T18:43:11Z

The failure was due to a timeout on travis.

@Dr-Irv : I think that's being handled in #23441 .

TomAugspurger · 2018-11-01T22:20:46Z

Merged master with #23441.

jreback

lgtm. questions on some tests changes that were needed.

jreback · 2018-11-02T14:10:07Z

pandas/tests/indexes/test_base.py

@@ -715,7 +715,10 @@ def test_empty_fancy_raises(self, attr):
    # FutureWarning from non-tuple sequence of nd indexing
    @pytest.mark.filterwarnings("ignore::FutureWarning")
    def test_getitem_error(self, indices, itm):
-        with pytest.raises(IndexError):
+        error_type = IndexError
+        if isinstance(indices, RangeIndex) and (itm == 'no_int'):


grr, why did this need changing?

One of the changes I did to testing was to add RangeIndex to the index types used for all of the base index tests. See the change in pandas/tests/indexes/conftest.py . When I did that, it revealed some differences in how RangeIndex works versus the other index types. For the above test, when you pass a string to a RangeIndex, pandas returns a ValueError as opposed to an IndexError, so the change was necessary.

Alternatively, we could change the behavior of RangeIndex to return IndexError when a string is passed to get_item.

if this fix is straightforward, pls let's just do this here

This one was straightforward.

jreback · 2018-11-02T14:10:31Z

pandas/tests/indexes/test_base.py

@@ -2517,6 +2581,10 @@ def test_generated_op_names(opname, indices):
        # pd.Index.__rsub__ does not exist; though the method does exist
        # for subclasses.  see GH#19723
        return
+    if (isinstance(index, RangeIndex) and
+        opname in ['add', 'radd', 'sub', 'rsub',


Same as above. Adding RangeIndex to pandas/tests/indexes/conftest.py revealed that the implementation of the operators for RangeIndex is done differently. Namely, for RangeIndex, the op methods are bound to one function with a parameter that includes the opname, but for the other index types, there are direct methods for each of the operators.

same comment as above. would like to fix rather than special case a test.

Was a little tricky, but I think I figured it out right.

jreback

if you can fix those would be great. ping on green.

jreback · 2018-11-03T14:24:44Z

pandas/tests/indexes/test_base.py

@@ -715,7 +715,10 @@ def test_empty_fancy_raises(self, attr):
    # FutureWarning from non-tuple sequence of nd indexing
    @pytest.mark.filterwarnings("ignore::FutureWarning")
    def test_getitem_error(self, indices, itm):
-        with pytest.raises(IndexError):
+        error_type = IndexError
+        if isinstance(indices, RangeIndex) and (itm == 'no_int'):


if this fix is straightforward, pls let's just do this here

jreback · 2018-11-03T14:25:03Z

pandas/tests/indexes/test_base.py

@@ -2517,6 +2581,10 @@ def test_generated_op_names(opname, indices):
        # pd.Index.__rsub__ does not exist; though the method does exist
        # for subclasses.  see GH#19723
        return
+    if (isinstance(index, RangeIndex) and
+        opname in ['add', 'radd', 'sub', 'rsub',


same comment as above. would like to fix rather than special case a test.

jreback · 2018-11-06T13:15:38Z

lgtm. @Dr-Irv ping on green.

Dr-Irv · 2018-11-06T16:20:52Z

@jreback all green

gfyoung · 2018-11-06T18:01:29Z

@Dr-Irv @jreback : Is this PR meant to also close #9943? (or are you just "xref"-ing it) . I only ask because you mention the issue number in the whatsnew but are not closing it.

Dr-Irv · 2018-11-06T18:16:16Z

@gfyoung Back when I created the PR, I thought this closed #9943 because #9943 was split into #9862 and #9885 . But then I see that @jreback edited the original description to xref #9943. #9943 and #9862 cross reference each other, so I thought putting both refs in the whatsnew is what made sense. But I'll do whatever you guys think is best.

gfyoung · 2018-11-06T18:21:16Z

Alright then. I'll merge, as is, and let @jreback decide what to do with #9943.

gfyoung · 2018-11-06T18:22:19Z

Thanks @Dr-Irv !

…das-dev#19849) Closes pandas-devgh-9862. xref pandas-devgh-9943.

gfyoung added Bug Indexing Related to indexing on series/frames, not to indexes themselves Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Feb 23, 2018

gfyoung reviewed Feb 23, 2018

View reviewed changes

pandas/core/indexes/base.py Outdated

name = self.name

return name

def _get_intersection_name(self, other):

Copy link

Member

gfyoung Feb 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docstring

crayxt reviewed Feb 23, 2018

View reviewed changes

Dr-Irv mentioned this pull request Feb 23, 2018

BUG: index name lost when indexing with another index #9943

Closed

jreback requested changes Feb 24, 2018

View reviewed changes

BUG: names on union and intersection for Index were inconsistent (GH9…

aad5884

…943 GH9862)

Dr-Irv force-pushed the issue9943 branch from 9058c53 to aad5884 Compare February 26, 2018 22:46

jreback requested changes Feb 27, 2018

View reviewed changes

Use _shallow_copy(). Fix issue in Index.difference. Make tests try va…

3ef2a69

…riety of index types.

jreback requested changes Mar 1, 2018

View reviewed changes

TomAugspurger reviewed Mar 2, 2018

View reviewed changes

Updated whatsnew

ba9ccdd

Remove set difference fix

38aeacb

Dr-Irv mentioned this pull request Mar 7, 2018

BUG: Index.difference and Index.intersection doesn't preserve type of Index for some Index subclasses for corner cases #20040

Closed

Only include changes relative to name preservation

fb3a9db

jreback requested changes Mar 7, 2018

View reviewed changes

Use parametrize in test

f72377a

jreback requested changes Mar 7, 2018

View reviewed changes

Dr-Irv added 2 commits March 8, 2018 14:14

remove is_corner_union and change parametrize on test

08eb291

Merge remote-tracking branch 'upstream/master' into issue9943

396088f

jbrockmendel reviewed Jul 11, 2018

View reviewed changes

merge in latest master

a030ede

Dr-Irv added 2 commits November 1, 2018 11:35

Merge with latest master

9ba6960

Merge with latest master

d1620f6

jreback requested changes Nov 2, 2018

View reviewed changes

jreback requested changes Nov 3, 2018

View reviewed changes

Dr-Irv added 2 commits November 5, 2018 15:02

Fix RangeIndex implementation to avoid test changes

893efb8

Merge to latest master

fd7f23f

Dr-Irv force-pushed the issue9943 branch from 0701fc8 to fd7f23f Compare November 5, 2018 20:42

Dr-Irv added 2 commits November 5, 2018 22:25

Merge remote-tracking branch 'upstream/master' into issue9943

2d1c366

fix isort on range.py

fa7311a

jreback approved these changes Nov 6, 2018

View reviewed changes

gfyoung merged commit 810826d into pandas-dev:master Nov 6, 2018

JustinZhengBC pushed a commit to JustinZhengBC/pandas that referenced this pull request Nov 14, 2018

BUG: names on union and intersection for Index were inconsistent (pan…

2acb22c

…das-dev#19849) Closes pandas-devgh-9862. xref pandas-devgh-9943.

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

BUG: names on union and intersection for Index were inconsistent (pan…

a3cb293

…das-dev#19849) Closes pandas-devgh-9862. xref pandas-devgh-9943.

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

BUG: names on union and intersection for Index were inconsistent (pan…

108f2ae

…das-dev#19849) Closes pandas-devgh-9862. xref pandas-devgh-9943.

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

BUG: names on union and intersection for Index were inconsistent (pan…

47239bd

…das-dev#19849) Closes pandas-devgh-9862. xref pandas-devgh-9943.

Dr-Irv deleted the issue9943 branch July 12, 2019 17:21

		@@ -707,6 +707,98 @@ def test_intersect_str_dates(self):

		assert len(res) == 0

		def create_empty_index(self, id):

BUG: names on union and intersection for Index were inconsistent (GH9943 GH9862) #19849

BUG: names on union and intersection for Index were inconsistent (GH9943 GH9862) #19849

Conversation

Dr-Irv commented Feb 22, 2018 • edited by jreback Loading

gfyoung commented Feb 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dr-Irv commented Feb 23, 2018

codecov bot commented Feb 24, 2018 • edited Loading

Codecov Report

jreback left a 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

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

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

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

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 left a 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

Choose a reason for hiding this comment

TomAugspurger left a 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

Dr-Irv commented Mar 2, 2018 • edited Loading

jreback commented Mar 7, 2018

Dr-Irv commented Mar 7, 2018

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

Dr-Irv commented Mar 7, 2018

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

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

jbrockmendel commented Jul 11, 2018

jreback commented Nov 1, 2018

pep8speaks commented Nov 1, 2018

Dr-Irv commented Feb 22, 2018 •

edited by jreback

Loading

codecov bot commented Feb 24, 2018 •

edited

Loading

Dr-Irv commented Mar 2, 2018 •

edited

Loading

gfyoung commented Nov 1, 2018 •

edited

Loading