BUG: List indexer on PeriodIndex doesn't coerce strings #30515

proost · 2019-12-27T18:38:13Z

closes List indexer on PeriodIndex doesn't coerce strings #11278
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Description:
In single object indexer case,
datetime string converted to datetime-like object. Because indexing use ".xs" which is ".get_loc" that has type casting labels code.

But in list indexer,
there is no type check and casting code. So i add them.

this bug happen not only series but also dataframe And "datetimeindex" as well as "periodIndex"

jreback

there are specific conversions for index types
this is adding a large special case

pls find a more existing code friendly way

indexing code is already very complex
we need to reduce that not add to it

doc/source/whatsnew/v1.0.0.rst

pandas/core/indexing.py

pep8speaks · 2020-01-03T17:34:34Z

Hello @proost! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-05-03 15:17:33 UTC

pandas/core/indexes/base.py

doc/source/whatsnew/v1.0.0.rst

pandas/core/common.py

jreback · 2020-01-26T01:42:34Z

pandas/core/indexes/datetimes.py

@@ -1006,6 +1006,16 @@ def indexer_between_time(

        return mask.nonzero()[0]

+    def _convert_arr_indexer(self, keyarr):


where exactly are these hit?

these function call from "._convert_listlike_indexer" and get returned value from "common.asarray_tuple"

jreback · 2020-01-26T01:42:55Z

pandas/core/indexes/period.py

@@ -839,6 +839,16 @@ def memory_usage(self, deep=False):
            result += self._int64index.memory_usage(deep=deep)
        return result

+    def _convert_arr_indexer(self, keyarr):


why would these not share the impl from the superclass?

At first, I Implemented on the base But,

would it be simpler to override DatetimeIndex._convert_arr_indexer and PeriodIndex._convert_arr_indexer?

So I changed. Okay I change again.

jreback · 2020-01-26T01:43:19Z

@jbrockmendel if you'd have a look

doc/source/whatsnew/v1.1.0.rst

jbrockmendel · 2020-01-28T20:35:56Z

pandas/core/common.py

+    if isinstance(values, list) and (
+        is_datetime64_any_dtype(dtype) or is_period_dtype(dtype)
+    ):
+        return array(values, dtype)


does timedelta64 ever go through here? could just check needs_i8_conversion(dtype)

What about other EA dtypes? isnt this kind of "anything that we dont want to pass to np.asarray below"?

@jbrockmendel
Okay. I change code

pandas/core/indexes/period.py

…0515)

jbrockmendel · 2020-02-19T19:16:02Z

pandas/core/common.py

@@ -225,6 +225,10 @@ def asarray_tuplesafe(values, dtype=None):
    if isinstance(values, list) and dtype in [np.object_, object]:
        return construct_1d_object_array_from_listlike(values)

+    if isinstance(values, list) and hasattr(values, "__array__"):
+        # avoid converting extension array to numpy array


so we're looking at some custom list subclass?

@jbrockmendel
"asarray_tuplesafe" and "_convert_arr_indexer" both return "array-like" object. before, "asarray_tuplesafe" and "_convert_arr_indexer" return numpy array type. But i think, both can return EA type also

why can't you use use is_list_like?

pandas/core/indexes/base.py

pandas/core/indexes/datetimelike.py

pandas/tests/indexing/test_loc.py

…0515)

jreback · 2020-04-19T22:20:18Z

@jbrockmendel over to you if any comments.

doc/source/whatsnew/v1.1.0.rst

pandas/core/indexes/datetimelike.py

jbrockmendel · 2020-04-20T01:06:36Z

pandas/core/indexes/datetimelike.py

+        if lib.infer_dtype(keyarr) == "string":
+            # Weak reasoning that indexer is a list of strings
+            # representing datetime or timedelta or period
+            extension_arr = pd_array(keyarr, self.dtype)


if we have strings that cant be parsed to self.dtype this raises DateParseError (at least in the Period[D] case). should we catch and re-raise as KeyError?

@jbrockmendel
Okay, It makes sense. I add exception handling and test code

…0515)

jreback · 2020-05-02T16:40:27Z

looks find. can you merge master and ping o green.

…0515)

proost · 2020-05-03T15:55:38Z

ping @jreback

jreback · 2020-05-03T16:09:28Z

thanks @proost for sticking with this

BUG: List indexer on PeriodIndex doesn't coerce strings (pandas-dev#30515) (#30…

…0515) (pandas-dev#30515)

jreback requested changes Dec 27, 2019

View reviewed changes

gfyoung added Bug Indexing Related to indexing on series/frames, not to indexes themselves Period Period data type labels Dec 27, 2019

gfyoung reviewed Dec 27, 2019

View reviewed changes

doc/source/whatsnew/v1.0.0.rst Outdated Show resolved Hide resolved

gfyoung reviewed Dec 27, 2019

View reviewed changes

pandas/core/indexing.py Outdated Show resolved Hide resolved

gfyoung reviewed Dec 27, 2019

View reviewed changes

pandas/core/indexing.py Outdated Show resolved Hide resolved

jbrockmendel reviewed Dec 31, 2019

View reviewed changes

pandas/core/indexing.py Outdated Show resolved Hide resolved

proost force-pushed the enh-loc-datetime-list branch 6 times, most recently from 0c9ee8c to 2cbd26b Compare January 3, 2020 17:34

proost force-pushed the enh-loc-datetime-list branch from 2cbd26b to 5e86611 Compare January 3, 2020 17:38

jbrockmendel reviewed Jan 12, 2020

View reviewed changes

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

proost force-pushed the enh-loc-datetime-list branch from a414a4f to fa51eb7 Compare January 14, 2020 14:44

jreback requested changes Jan 26, 2020

View reviewed changes

proost force-pushed the enh-loc-datetime-list branch 3 times, most recently from 2805c8d to 8b9d712 Compare January 28, 2020 14:55

jbrockmendel reviewed Jan 28, 2020

View reviewed changes

doc/source/whatsnew/v1.1.0.rst Outdated Show resolved Hide resolved

jbrockmendel reviewed Jan 28, 2020

View reviewed changes

pandas/core/indexes/period.py Outdated Show resolved Hide resolved

proost added a commit to proost/pandas that referenced this pull request Feb 19, 2020

BUG: List indexer on PeriodIndex doesn't coerce strings (pandas-dev#3…

91f70fc

…0515)

proost force-pushed the enh-loc-datetime-list branch from 8b9d712 to 91f70fc Compare February 19, 2020 16:39

jbrockmendel reviewed Feb 19, 2020

View reviewed changes

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

proost force-pushed the enh-loc-datetime-list branch from cb53b55 to fa76233 Compare April 1, 2020 07:52

jreback requested changes Apr 10, 2020

View reviewed changes

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

pandas/tests/indexing/test_loc.py Outdated Show resolved Hide resolved

proost added a commit to proost/pandas that referenced this pull request Apr 12, 2020

BUG: List indexer on PeriodIndex doesn't coerce strings (pandas-dev#3…

3ae8f48

…0515)

proost force-pushed the enh-loc-datetime-list branch from fa76233 to 3ae8f48 Compare April 12, 2020 16:36

proost added a commit to proost/pandas that referenced this pull request Apr 14, 2020

BUG: List indexer on PeriodIndex doesn't coerce strings (pandas-dev#3…

b138f94

…0515)

proost force-pushed the enh-loc-datetime-list branch from 3ae8f48 to b138f94 Compare April 14, 2020 03:26

proost added a commit to proost/pandas that referenced this pull request Apr 14, 2020

BUG: List indexer on PeriodIndex doesn't coerce strings (pandas-dev#3…

dbb0cdd

…0515)

proost force-pushed the enh-loc-datetime-list branch from b138f94 to dbb0cdd Compare April 14, 2020 05:02

jreback added this to the 1.1 milestone Apr 19, 2020

jreback approved these changes Apr 19, 2020

View reviewed changes

jbrockmendel reviewed Apr 20, 2020

View reviewed changes

doc/source/whatsnew/v1.1.0.rst Outdated Show resolved Hide resolved

jbrockmendel reviewed Apr 20, 2020

View reviewed changes

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

jbrockmendel reviewed Apr 20, 2020

View reviewed changes

proost added a commit to proost/pandas that referenced this pull request Apr 20, 2020

BUG: List indexer on PeriodIndex doesn't coerce strings (pandas-dev#3…

3efb005

…0515)

proost force-pushed the enh-loc-datetime-list branch from f9f246b to 3efb005 Compare April 20, 2020 11:09

proost added a commit to proost/pandas that referenced this pull request Apr 20, 2020

BUG: List indexer on PeriodIndex doesn't coerce strings (pandas-dev#3…

c143ddb

…0515)

proost force-pushed the enh-loc-datetime-list branch 2 times, most recently from ac86f36 to c143ddb Compare April 20, 2020 14:50

BUG: List indexer on PeriodIndex doesn't coerce strings (pandas-dev#3…

9f2db9a

…0515)

proost force-pushed the enh-loc-datetime-list branch from 7e57054 to 9f2db9a Compare May 3, 2020 15:17

jreback approved these changes May 3, 2020

View reviewed changes

jreback merged commit a3477c7 into pandas-dev:master May 3, 2020

proost deleted the enh-loc-datetime-list branch May 3, 2020 16:49

sthagen added a commit to sthagen/pandas-dev-pandas that referenced this pull request May 3, 2020

Merge pull request #111 from pandas-dev/master

aa68749

BUG: List indexer on PeriodIndex doesn't coerce strings (pandas-dev#30515) (#30…

rhshadrach pushed a commit to rhshadrach/pandas that referenced this pull request May 10, 2020

BUG: List indexer on PeriodIndex doesn't coerce strings (pandas-dev#3…

7760ee3

…0515) (pandas-dev#30515)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: List indexer on PeriodIndex doesn't coerce strings #30515

BUG: List indexer on PeriodIndex doesn't coerce strings #30515

proost commented Dec 27, 2019 •

edited

Loading

jreback left a comment

pep8speaks commented Jan 3, 2020 •

edited

Loading

jreback Jan 26, 2020

proost Jan 28, 2020

jreback Jan 26, 2020

proost Jan 28, 2020

jreback commented Jan 26, 2020

jbrockmendel Jan 28, 2020

proost Feb 19, 2020

jbrockmendel Feb 19, 2020

proost Feb 20, 2020

jreback Feb 23, 2020

jreback commented Apr 19, 2020

jbrockmendel Apr 20, 2020

proost Apr 20, 2020

jreback commented May 2, 2020

proost commented May 3, 2020

jreback commented May 3, 2020

		@@ -1006,6 +1006,16 @@ def indexer_between_time(

		return mask.nonzero()[0]

		def _convert_arr_indexer(self, keyarr):

BUG: List indexer on PeriodIndex doesn't coerce strings #30515

BUG: List indexer on PeriodIndex doesn't coerce strings #30515

Conversation

proost commented Dec 27, 2019 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

pep8speaks commented Jan 3, 2020 • edited Loading

Comment last updated at 2020-05-03 15:17:33 UTC

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 Jan 26, 2020

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 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented May 2, 2020

proost commented May 3, 2020

jreback commented May 3, 2020

proost commented Dec 27, 2019 •

edited

Loading

pep8speaks commented Jan 3, 2020 •

edited

Loading