BUG: TDI.insert with empty TDI raising IndexError #30757
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@simonjayhawkins suggestions for fixing mypy? |
|
am AFK for the rest of the day (Pydata London). will look tomorrow. |
jreback
approved these changes
Jan 8, 2020
pandas/core/indexes/datetimelike.py
Outdated
| @@ -96,7 +96,7 @@ class DatetimeIndexOpsMixin(ExtensionIndex, ExtensionOpsMixin): | |||
| Common ops mixin to support a unified interface datetimelike Index. | |||
| """ | |||
|
|
|||
| _data: ExtensionArray | |||
| _data: Union[DatetimeArray, TimedeltaArray, PeriodArray] | |||
There was a problem hiding this comment.
should define this with a short label, maybe DatetimeArrayType
pandas/core/indexes/datetimes.py
Outdated
| @@ -946,7 +945,7 @@ def insert(self, loc, item): | |||
| freq = self.freq | |||
| elif (loc == len(self)) and item - self.freq == self[-1]: | |||
| freq = self.freq | |||
| item = _to_M8(item, tz=self.tz) | |||
| item = item.asm8 | |||
There was a problem hiding this comment.
can we remove _to_M8?
IIRC there's still one more usage to get rid of after this. I'm eager to get rid of it, as im pretty sure it is hiding a bunch of corner case bugs.
| idx[:0].insert(0, td) | ||
| idx[:0].insert(1, td) | ||
| idx[:0].insert(-1, td) | ||
|
|
There was a problem hiding this comment.
maybe worth comparing the results
|
ping on green. |
|
ping |
|
can you rebase |
…f-insert-cosmetic
|
thanks @jbrockmendel |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This started out as a cosmetic-only branch and ended up finding a broken corner case. The relevant change is in timedeltas L416 where
if self.freq is not None:is nowif self.size and self.freq is not None:Using _check_compatible_with causes us to raise TypeError instead of ValueError in a couple of the DatetimeIndex.insert cases.