-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
fix inconsistent index naming with union/intersect #35847 #36413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -719,33 +719,29 @@ def intersection(self, other, sort=False): | |
""" | ||
self._validate_sort_keyword(sort) | ||
self._assert_can_do_setop(other) | ||
res_name = get_op_result_name(self, other) | ||
|
||
if self.equals(other): | ||
return self._get_reconciled_name_object(other) | ||
|
||
if len(self) == 0: | ||
return self.copy() | ||
return self.copy()._get_reconciled_name_object(other) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in the base class you got rid of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still keeping this around for the rare case where you want to return the same values but with a possible name change. In base.py it only shows up in the one |
||
if len(other) == 0: | ||
return other.copy() | ||
return other.copy()._get_reconciled_name_object(self) | ||
jreback marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if not isinstance(other, type(self)): | ||
result = Index.intersection(self, other, sort=sort) | ||
if isinstance(result, type(self)): | ||
if result.freq is None: | ||
# TODO: no tests rely on this; needed? | ||
result = result._with_freq("infer") | ||
result.name = res_name | ||
return result | ||
|
||
elif not self._can_fast_intersect(other): | ||
result = Index.intersection(self, other, sort=sort) | ||
# We need to invalidate the freq because Index.intersection | ||
# uses _shallow_copy on a view of self._data, which will preserve | ||
# self.freq if we're not careful. | ||
result = result._with_freq(None)._with_freq("infer") | ||
result.name = res_name | ||
return result | ||
return result._with_freq(None)._with_freq("infer") | ||
|
||
# to make our life easier, "sort" the two ranges | ||
if self[0] <= other[0]: | ||
|
@@ -759,11 +755,13 @@ def intersection(self, other, sort=False): | |
start = right[0] | ||
|
||
if end < start: | ||
return type(self)(data=[], dtype=self.dtype, freq=self.freq, name=res_name) | ||
result = type(self)(data=[], dtype=self.dtype, freq=self.freq) | ||
else: | ||
lslice = slice(*left.slice_locs(start, end)) | ||
left_chunk = left._values[lslice] | ||
return type(self)._simple_new(left_chunk, name=res_name) | ||
result = type(self)._simple_new(left_chunk) | ||
|
||
return self._wrap_setop_result(other, result) | ||
|
||
def _can_fast_intersect(self: _T, other: _T) -> bool: | ||
if self.freq is None: | ||
|
@@ -858,7 +856,7 @@ def _fast_union(self, other, sort=None): | |
# The can_fast_union check ensures that the result.freq | ||
# should match self.freq | ||
dates = type(self._data)(dates, freq=self.freq) | ||
result = type(self)._simple_new(dates, name=self.name) | ||
result = type(self)._simple_new(dates) | ||
return result | ||
else: | ||
return left | ||
|
@@ -883,8 +881,8 @@ def _union(self, other, sort): | |
result = result._with_freq("infer") | ||
return result | ||
else: | ||
i8self = Int64Index._simple_new(self.asi8, name=self.name) | ||
i8other = Int64Index._simple_new(other.asi8, name=other.name) | ||
i8self = Int64Index._simple_new(self.asi8) | ||
i8other = Int64Index._simple_new(other.asi8) | ||
i8result = i8self._union(i8other, sort=sort) | ||
result = type(self)(i8result, dtype=self.dtype, freq="infer") | ||
return result | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add the issue reference here.
Also I would add the same text here in the docs, maybe in the reshaping section, in a note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, i ended up adding it as a note in the "merge, join, concatenate, and compare" section.