API: warning to raise KeyError in the future if not all elements of a list are selected via .loc #17295

jreback · 2017-08-20T19:58:26Z

jreback · 2017-08-20T19:58:36Z

still lots of warnings.

codecov · 2017-08-20T21:13:46Z

Codecov Report

Merging #17295 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #17295      +/-   ##
==========================================
- Coverage   91.25%   91.24%   -0.02%     
==========================================
  Files         163      163              
  Lines       49859    49866       +7     
==========================================
  Hits        45499    45499              
- Misses       4360     4367       +7

Flag	Coverage Δ
#multiple	`89.04% <100%> (ø)`	⬆️
#single	`40.24% <21.42%> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexing.py	`93.97% <100%> (+0.03%)`	⬆️
pandas/core/series.py	`95.04% <100%> (ø)`	⬆️
pandas/io/formats/excel.py	`96.69% <100%> (+0.04%)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.73% <0%> (-0.1%)`	⬇️
pandas/core/internals.py	`94.37% <0%> (-0.01%)`	⬇️
pandas/plotting/_style.py	`74.28% <0%> (ø)`	⬆️
pandas/core/indexes/base.py	`96.46% <0%> (+0.11%)`	⬆️

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 2e2093e...44ce09d. Read the comment docs.

jreback · 2017-08-22T12:12:02Z

ok this is ready to go.

@jorisvandenbossche @TomAugspurger
cc @toobaz

toobaz · 2017-08-22T12:19:20Z

doc/source/whatsnew/v0.21.0.txt

+
+   s.loc[[1, 2]]
+
+Previously Behavior


Previous Behavior

toobaz · 2017-08-22T12:43:31Z

Cool!

Notice however that the warning is not being raised for MultiIndexes.

In [2]: pd.DataFrame([[i] for i in range(3)], index=pd.MultiIndex.from_product([[1], [5,6,7]])).loc[[(1,5), (3,7)]]
Out[2]: 
       0
1 5  0.0
3 7  NaN

I assume/hope that we will want to change the behavior with partial indexing too (despite the current behaviour being different):

In [3]: pd.DataFrame([[i] for i in range(3)], index=pd.MultiIndex.from_product([[1], [5,6,7]])).loc[[1,3]]
Out[3]: 
     0
1 5  0
  6  1
  7  2

So maybe this too should raise a warning (although the whatsnew notes would maybe require some addition). Can go in another PR however.

jreback · 2017-08-22T12:44:54Z

So maybe this too should raise a warning (although the whatsnew notes would maybe require some addition). Can go in another PR however.

yeah this doesn't cover MultiIndex cases at all, but agree it should/could. welcome for PR's on top.

toobaz · 2017-08-22T12:59:54Z

One more thought: users might have in their code (I suspect I do) things like

a.loc[list_like].dropna()

... in which case the suggested replacement is not a.reindex(list_like).dropna() but rather a.loc[a.index.intersection(list_like)] (which avoids a step, and possibly a type casting of ints to floats). Should this second option maybe be mentioned?

jreback · 2017-08-22T13:07:25Z

@toobaz in your comment #17295 (comment)

yes I think the warning could refer to a new section in the docs (like we do for .ix). where a more extended example could be. (and include this effect as well). let me add.

jreback · 2017-08-22T13:45:29Z

updated with nicer message and doc section.

toobaz · 2017-08-23T20:59:46Z

doc/source/indexing.rst

+
+.. warning::
+
+   Starting in 0.21.0, using ``.loc`` with missing keys in a list. This is deprecated, in favor of ``.reindex``.


"using .loc with missing keys in a list. This is deprecated" -> "using .loc with a list-like containing missing keys"?

toobaz · 2017-08-23T21:01:37Z

doc/source/indexing.rst

+  s.reindex([1, 2, 3])
+
+
+Alternatively if you want to selected only *valid* keys, the following is idiomatic; furthermore this will preserve the dtype of the selection.


want to select

maybe I would also replace "this will preserve the dtype of the selection" with "this is more efficient, and is guaranteed to preserve the dtype of the selection"

toobaz · 2017-08-23T21:10:32Z

I think it is OK, just the two sentences commented above and the "Previously Behavior" sound a bit weird.

jreback · 2017-08-23T23:23:52Z

ok pushed. @jorisvandenbossche @TomAugspurger if you'd have a look.

TomAugspurger

Looks good. One concern I just realized is we may be stranding users with duplicates in their index.

In [15]: s = pd.Series(1, index=['a', 'a', 'b', 'c'])

In [16]: s.loc[['c', 'd']]
Out[16]:
c    1.0
d    NaN
dtype: float64

In [17]: s.reindex(['c', 'd'])
...
ValueError: cannot reindex from a duplicate axis

I haven't kept up with whether reindexing into a Index with dupes is supposed to work on not...

TomAugspurger · 2017-08-24T01:55:42Z

doc/source/indexing.rst

+
+.. warning::
+
+   Starting in 0.21.0, using ``.loc`` with a list-like containing missing keys. This is deprecated, in favor of ``.reindex``.


Can remove “this”

TomAugspurger · 2017-08-24T01:59:44Z

doc/source/indexing.rst

+   s = Series([1, 2, 3])
+   s
+
+Selection with all keys found is unchanged.


I would move this one to the end. Good to have in this section though.

TomAugspurger · 2017-08-24T02:01:27Z

doc/source/indexing.rst

+Current Behavior
+
+   In [4]: s.loc[[1, 2, 3]]
+   /Users/jreback/miniconda3/envs/pandas/bin/ipython:1: FutureWarning: passing list-likes to .loc with any non-matching elements will raise


You could remove most of your user path here

TomAugspurger · 2017-08-24T02:02:05Z

doc/source/whatsnew/v0.21.0.txt

+   3    NaN
+   dtype: float64
+
+The idiomatic way to achieve selecting potentially not-found elmenents if via ``.reindex()``


toobaz · 2017-08-24T07:11:16Z

I haven't kept up with whether reindexing into a Index with dupes is supposed to work on not...

Indeed, it can't work in general - assume you had done s.reindex(['a', 'b']), the expected behavior would be unclear because 'a' is duplicated.

So maybe the s.loc[s.index.intersection(keys)] option (or does s.loc[s.index & keys] look better?) should be emphasized for solving this case too. To my eyes, it is actually the idiomatic replacement, because it is hard to conceive that one would use s.loc[list_like] and be interested in keys not in s...

toobaz · 2017-08-24T10:02:19Z

(Edited my previous comment, which in that form didn't make any sense)

jreback · 2017-08-24T10:14:54Z

So maybe the s.loc[s.index.intersection(keys)] option (or does s.loc[s.index & keys] look better?) should be emphasized for solving this case too. To my eyes, it is actually the idiomatic replacement, because it is hard to conceive that one would use s.loc[list_like] and be interested in keys not in s...

changed; that is quite idiomatic

jreback · 2017-08-24T10:26:32Z

fixed according to @TomAugspurger and @toobaz comments.

jorisvandenbossche · 2017-09-24T09:39:24Z

(Will take a look at this tomorrow evening)

jorisvandenbossche

Only did a new pass through the docs, will look at the rest tomorrow.

jorisvandenbossche · 2017-09-25T22:22:51Z

doc/source/indexing.rst

@@ -333,8 +333,15 @@ Selection By Label

     dfl.loc['20130102':'20130104']

+.. warning::
+
+   Starting in 0.21.0, pandas will show a ``FutureWarning`` if indexing with a list-of-lables and not ALL labels are present. In the future


lables -> labels

still al 'lables' -> labels (the first occurrence of the word)

jorisvandenbossche · 2017-09-25T22:23:42Z

doc/source/indexing.rst

+.. warning::
+
+   Starting in 0.21.0, pandas will show a ``FutureWarning`` if indexing with a list-of-lables and not ALL labels are present. In the future
+   this will raise a ``KeyError``. See :ref:`list-like Using loc with missing keys in a list is Deprecated <indexing.deprecate_loc_reindex_listlike>`


minor: Using -> using (a capital in the middle of the sentence is a bit strange)

the text within the ref does not read as grammatically correct. Maybe the 'list-like' needs to be removed? (list is mentioned later in the sentence)

jorisvandenbossche · 2017-09-25T22:24:21Z

doc/source/indexing.rst

+
+.. warning::
+
+   Starting in 0.21.0, using ``.loc`` with a list-like containing missing key, is deprecated, in favor of ``.reindex``.


not only loc, also []

jorisvandenbossche · 2017-09-25T22:24:52Z

doc/source/indexing.rst

+
+.. ipython:: python
+
+   s = Series([1, 2, 3])


jorisvandenbossche · 2017-09-25T22:29:36Z

doc/source/indexing.rst

+
+.. ipython:: python
+
+   s.loc[s.index & ['c', 'd']]


Repeating my comment:

I would use intersection instead of &

this is not a correct example (the alternative you provide does not give the same result as the deprecated one. As I say above I am not sure if there is actually an alternative, but if there is not, we should just remove this example)

jorisvandenbossche · 2017-09-25T22:31:59Z

doc/source/whatsnew/v0.21.0.txt

+
+Previously, selecting at least 1 valid key with a list-like indexer would succeed and return ``NaN`` for non-found elements.
+This is exactly the function of ``.reindex()``. This will now show a ``FutureWarning`` message; in the future this will raise ``KeyError`` (:issue:`15747`).
+This warning will trigger on a ``DataFrame`` or a ``Series`` for using ``.loc[]`` on the Index, or ``[[]]`` on a ``Series``.


I would remove on the Index, as you don't call loc on the index.

Also, maybe we could just say that it is triggered for both .loc[] and [] (without needing to make the distinction of dataframe and series. Could mention after that that [] on dataframe already raises KeyError now)

jorisvandenbossche · 2017-09-25T22:33:33Z

pandas/core/indexing.py

+                        # code, so we want to avoid warning & then
+                        # just raising
+                        _missing_key_warning = textwrap.dedent("""
+                        Passing list-likes to .loc with any non-matching labels will raise


not only .loc, also []
(now you get this error message also when using s[], which will be confusing)

jreback · 2017-09-26T10:29:33Z

updated

jreback · 2017-10-01T15:44:41Z

@jorisvandenbossche @TomAugspurger

jorisvandenbossche

Code looks good!
Added another bunch of doc comments.

Also, there are still two warnings in the docs (I didn't check whether you just need to add :okwarning: or rather need to update the example):

>>>-------------------------------------------------------------------------

Warning in /tmp/doc/source/advanced.rst at block ending on line 1015

Specify :okwarning: as an option in the ipython:: block to suppress this message

----------------------------------------------------------------------------

/home/travis/build/pandas-dev/pandas/pandas/core/series.py:694: FutureWarning: 

Passing list-likes to .loc or [] with any missing label will raise

KeyError in the future, you can use .reindex() as an alternative.

See the documentation here:

http://pandas.pydata.org/pandas-docs/stable/indexing.html#deprecate-loc-reindex-listlike

  return self.loc[key]

<<<-------------------------------------------------------------------------

>>>-------------------------------------------------------------------------

Warning in /tmp/doc/source/indexing.rst at block ending on line 963

Specify :okwarning: as an option in the ipython:: block to suppress this message

----------------------------------------------------------------------------

/home/travis/build/pandas-dev/pandas/pandas/core/series.py:694: FutureWarning: 

Passing list-likes to .loc or [] with any missing label will raise

KeyError in the future, you can use .reindex() as an alternative.

See the documentation here:

http://pandas.pydata.org/pandas-docs/stable/indexing.html#deprecate-loc-reindex-listlike

  return self.loc[key]

<<<-------------------------------------------------------------------------

jorisvandenbossche · 2017-10-01T18:10:03Z

doc/source/indexing.rst

@@ -333,8 +333,15 @@ Selection By Label

     dfl.loc['20130102':'20130104']

+.. warning::
+
+   Starting in 0.21.0, pandas will show a ``FutureWarning`` if indexing with a list-of-lables and not ALL labels are present. In the future


still al 'lables' -> labels (the first occurrence of the word)

jorisvandenbossche · 2017-10-01T18:10:30Z

doc/source/indexing.rst

+.. warning::
+
+   Starting in 0.21.0, pandas will show a ``FutureWarning`` if indexing with a list-of-lables and not ALL labels are present. In the future
+   this will raise a ``KeyError``. See :ref:`list-like Using loc with missing keys in a list is Deprecated <indexing.deprecate_loc_reindex_listlike>`


the text within the ref does not read as grammatically correct. Maybe the 'list-like' needs to be removed? (list is mentioned later in the sentence)

jorisvandenbossche · 2017-10-01T18:48:24Z

doc/source/indexing.rst

+
+Current Behavior
+
+   In [4]: s.loc[[1, 2, 3]]


missing .. code-block:: ipython

jorisvandenbossche · 2017-10-01T18:48:38Z

doc/source/indexing.rst

+   s = pd.Series(np.arange(4), index=['a', 'a', 'b', 'c'])
+   labels = ['c', 'd']
+
+.. code-block:: python


python -> ipython

jorisvandenbossche · 2017-10-01T18:48:41Z

doc/source/indexing.rst

+
+However, this would *still* raise if your resulting index is duplicated.
+
+.. code-block:: python


python -> ipython

jorisvandenbossche · 2017-10-01T18:50:14Z

doc/source/whatsnew/v0.21.0.txt

@@ -268,6 +268,63 @@ We have updated our minimum supported versions of dependencies (:issue:`15206`,
   | Bottleneck   | 1.0.0           |          |
   +--------------+-----------------+----------+

+.. _whatsnew_0210.api_breaking.loc:
+
+Indexing with missing list-of-labels is Deprecated


maybe "Indexing with list with missing labels .. " ? (I find the 'missing list-of-labels' a bit confusing)

jorisvandenbossche · 2017-10-01T18:53:57Z

doc/source/whatsnew/v0.21.0.txt

+
+Current Behavior
+
+   In [4]: s.loc[[1, 2, 3]]


missing .. code-block:: ipython

jorisvandenbossche · 2017-10-01T18:59:26Z

pandas/io/formats/excel.py

+
+            # all missing, raise
+            if not len(Index(cols) & df.columns):
+                raise KeyError


can you provide an error message?

jorisvandenbossche · 2017-10-01T19:00:44Z

pandas/io/formats/excel.py

+            # 1 missing is ok
+            # TODO(jreback) this should raise
+            # on *any* missing columns
+            self.df = df.reindex(columns=cols)


This indeed currently works, but I think it is certainly OK to change that. Or maybe also raise futurewarning as with loc?

jreback · 2017-10-01T23:26:00Z

push to fix all @jorisvandenbossche comments. I deprecated passing 1 or more missing .to_excel(..., columns=)

jorisvandenbossche

Minor comment about wording in error message. For the rest good to merge!

jorisvandenbossche · 2017-10-02T19:43:17Z

pandas/io/formats/excel.py

+                warnings.warn(
+                    "columns must be a subset of the "
+                    "dataframe columns; this will raise "
+                    "a KeyError in the future",


When you only see that error message, it is not directly clear that on the the names you passed in columns is missing (you directly say the result). Can you clarify that?
Maybe like "Not all names specified in 'columns' are found(or present); this will raise a KeyError in the future"

jreback · 2017-10-02T19:55:42Z

ok pushed

… list are selected via .loc closes pandas-dev#15747

jorisvandenbossche · 2017-10-04T11:29:21Z

This is really nice, thanks @jreback !

… list are selected via .loc (pandas-dev#17295) closes pandas-dev#15747

jreback added Deprecate Functionality to remove in pandas Indexing Related to indexing on series/frames, not to indexes themselves labels Aug 20, 2017

jreback force-pushed the key branch 3 times, most recently from 4b523f4 to e5f80e3 Compare August 22, 2017 12:11

jreback changed the title ~~WIP/API: warning to raise KeyError in the future if not all elements of a list are selected via .loc~~ API: warning to raise KeyError in the future if not all elements of a list are selected via .loc Aug 22, 2017

jreback added this to the 0.21.0 milestone Aug 22, 2017

jsexauer mentioned this pull request Aug 22, 2017

DEPR: Clean up list of deprecations from prior versions #6581

Closed

1 task

toobaz reviewed Aug 22, 2017

View reviewed changes

jreback force-pushed the key branch from e5f80e3 to a5c7e2a Compare August 22, 2017 13:45

TomAugspurger mentioned this pull request Aug 22, 2017

RLS: 1.0 #17287

Closed

6 tasks

toobaz reviewed Aug 23, 2017

View reviewed changes

jreback force-pushed the key branch from a5c7e2a to 8509dc6 Compare August 23, 2017 23:23

TomAugspurger reviewed Aug 24, 2017

View reviewed changes

jreback force-pushed the key branch from 8509dc6 to 22b1f4e Compare August 24, 2017 10:26

jorisvandenbossche reviewed Sep 25, 2017

View reviewed changes

jreback force-pushed the key branch from 62e19b2 to e527140 Compare September 26, 2017 10:29

jreback force-pushed the key branch from e527140 to e531e9a Compare October 1, 2017 16:33

jorisvandenbossche reviewed Oct 1, 2017

View reviewed changes

jreback force-pushed the key branch from e531e9a to 19c1de1 Compare October 1, 2017 23:25

jreback modified the milestone: 0.21.0 Oct 2, 2017

jreback force-pushed the key branch 2 times, most recently from a4fb72c to db52a4f Compare October 2, 2017 16:42

jorisvandenbossche approved these changes Oct 2, 2017

View reviewed changes

jorisvandenbossche mentioned this pull request Oct 2, 2017

Proposal to change behaviour with .loc and missing keys #15747

Closed

jreback force-pushed the key branch from db52a4f to bf02812 Compare October 2, 2017 19:55

toobaz mentioned this pull request Oct 3, 2017

No warning is raised by MultiIndex when .loc is called with list containing missing keys #17758

Closed

API: warning to raise KeyError in the future if not all elements of a…

44ce09d

… list are selected via .loc closes pandas-dev#15747

jreback force-pushed the key branch from bf02812 to 44ce09d Compare October 3, 2017 10:08

jreback merged commit 8e89cb3 into pandas-dev:master Oct 3, 2017

toobaz mentioned this pull request Oct 15, 2017

ERR: setting a column with a scalar and no index should raise #16823

Closed

ghost pushed a commit to reef-technologies/pandas that referenced this pull request Oct 16, 2017

API: warning to raise KeyError in the future if not all elements of a…

6ac291a

… list are selected via .loc (pandas-dev#17295) closes pandas-dev#15747

jreback mentioned this pull request Oct 21, 2017

ERR: better error message on non-sorted input with .truncate #17935

Closed

TomAugspurger mentioned this pull request Oct 25, 2017

PERF: Regressions since 0.20.3 #17861

Closed

alanbato pushed a commit to alanbato/pandas that referenced this pull request Nov 10, 2017

API: warning to raise KeyError in the future if not all elements of a…

688ac4c

… list are selected via .loc (pandas-dev#17295) closes pandas-dev#15747

No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017

API: warning to raise KeyError in the future if not all elements of a…

b1f8e3d

… list are selected via .loc (pandas-dev#17295) closes pandas-dev#15747

gfyoung mentioned this pull request Aug 2, 2018

Specific Timestamps breaks time series indexing (.loc returns wrong results) #18029

Closed

jreback mentioned this pull request Nov 29, 2019

DEPR: deprecations log for removed issues #13777

Closed


		.. warning::

		Starting in 0.21.0, using ``.loc`` with missing keys in a list. This is deprecated, in favor of ``.reindex``.

		s.reindex([1, 2, 3])


		Alternatively if you want to selected only valid keys, the following is idiomatic; furthermore this will preserve the dtype of the selection.


		However, this would still raise if your resulting index is duplicated.

		.. code-block:: python

API: warning to raise KeyError in the future if not all elements of a list are selected via .loc #17295

API: warning to raise KeyError in the future if not all elements of a list are selected via .loc #17295

Conversation

jreback commented Aug 20, 2017

jreback commented Aug 20, 2017

codecov bot commented Aug 20, 2017 • edited Loading

Codecov Report

jreback commented Aug 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toobaz commented Aug 22, 2017

jreback commented Aug 22, 2017

toobaz commented Aug 22, 2017

jreback commented Aug 22, 2017

jreback commented Aug 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toobaz commented Aug 23, 2017 • edited Loading

jreback commented Aug 23, 2017

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

toobaz commented Aug 24, 2017 • edited Loading

toobaz commented Aug 24, 2017 • edited Loading

jreback commented Aug 24, 2017

jreback commented Aug 24, 2017

jorisvandenbossche commented Sep 24, 2017

jorisvandenbossche 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

jreback commented Sep 26, 2017

jreback commented Oct 1, 2017

jorisvandenbossche 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

jreback commented Oct 1, 2017

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Oct 2, 2017

jorisvandenbossche commented Oct 4, 2017

codecov bot commented Aug 20, 2017 •

edited

Loading

toobaz commented Aug 23, 2017 •

edited

Loading

toobaz commented Aug 24, 2017 •

edited

Loading

toobaz commented Aug 24, 2017 •

edited

Loading