Support dicts with default values in series.map #16002

dhimmel · 2017-04-14T19:43:47Z

Still a work in progress.

jreback · 2017-04-14T19:54:36Z

pandas/core/series.py

-        if isinstance(arg, (dict, Series)):
-            if isinstance(arg, dict):
-                arg = self._constructor(arg, index=arg.keys())
+        default_dict_types = collections.Counter, collections.defaultdict


instead of actually naming these, use issubclass(type(arg), dict) and not type(arg) is dict

In [17]: f = lambda arg: issubclass(type(arg), dict) and not type(arg) is dict In [18]: f({}) Out[18]: False In [19]: f(collections.defaultdict()) Out[19]: True In [21]: f(collections.Counter()) Out[21]: True

issubclass(type(arg), dict) and not type(arg) is dict

This would evaluate as True for a collections.OrderedDict which should behave like a plain dict.

jreback · 2017-04-14T19:54:51Z

always write the tests first!

codecov · 2017-04-14T20:02:08Z

Codecov Report

Merging #16002 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #16002      +/-   ##
==========================================
+ Coverage   91.02%   91.02%   +<.01%     
==========================================
  Files         145      145              
  Lines       50391    50394       +3     
==========================================
+ Hits        45870    45873       +3     
  Misses       4521     4521

Flag	Coverage Δ
#multiple	`88.83% <100%> (ø)`	⬆️
#single	`40.33% <40%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/series.py	`95.09% <100%> (+0.01%)`	⬆️

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 2d9451d...79cfd11. Read the comment docs.

dhimmel · 2017-04-14T20:07:32Z

always write the tests first!

@jreback good idea (assuming this is so we can check that the tests fail before the enhancement).

I rebased to put the testing commit first. Unfortunately, the GitHub PR displays orders by commit time rather than order. Also CI is only building the tip commit.

jreback · 2017-04-14T20:08:40Z

u need to duck type the test rather than hard coding things

dhimmel · 2017-04-14T20:27:08Z

u need to duck type the test rather than hard coding things

I dug a bit deeper. The __missing__ method is how dict subclasses can return default values. Switched to this method in 961ea46.

https://docs.python.org/3.6/reference/datamodel.html#object.__missing__

jreback · 2017-04-14T20:34:44Z

great

make sure to test a dict subclass with and w/o missimg

document in doc string

add a note in other api changes section

jreback · 2017-04-14T21:02:44Z

pandas/core/series.py

@@ -2132,10 +2132,14 @@ def map_f(values, f):
            else:
                map_f = lib.map_infer

-        if isinstance(arg, (dict, Series)):
-            if isinstance(arg, dict):
+        if isinstance(arg, dict):


can you add a 1-line comment here on what you are doing

jreback · 2017-04-14T21:39:15Z

doc/source/whatsnew/v0.20.0.txt

@@ -449,10 +449,8 @@ Other Enhancements
 - Integration with the ``feather-format``, including a new top-level ``pd.read_feather()`` and ``DataFrame.to_feather()`` method, see :ref:`here <io.feather>`.
 - ``Series.str.replace()`` now accepts a callable, as replacement, which is passed to ``re.sub`` (:issue:`15055`)
 - ``Series.str.replace()`` now accepts a compiled regular expression as a pattern (:issue:`15446`)
-
-
+- ``Series.map()`` now respects default values of dictionary subclasses with a ``__missing__`` method, such as ``collections.Counter`` (:issue:`15999`, :issue:`16002`)


only list the issue (and not the PR)

this is an api change (and not an enhancement), ignore how I labelled the issue.

I moved the note to "Other API Changes". Is this okay, or should I make a larger entry under:

Backwards incompatible API changes ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

jreback · 2017-04-14T21:41:20Z

pandas/core/series.py

+        if isinstance(arg, dict):
+            if hasattr(arg, '__missing__'):
+                # If a dictionary subclass defines a default value method,
+                # convert arg to a lookup function (https://git.io/vS7LK).


you can just add GH #15999 instead of a short url

jreback · 2017-04-14T21:41:40Z

small comments, other lgtm.

dhimmel · 2017-04-14T22:09:44Z

Confirmed locally that the following tests fail under the old code for Series.map by running py.test --quiet pandas/tests/series/test_apply.py:

TestSeriesMap.test_map_counter
TestSeriesMap.test_map_defaultdict
TestSeriesMap.test_map_dict_subclass_with_missing

Removing WIP from title.

dhimmel · 2017-04-14T22:11:36Z

doc/source/whatsnew/v0.20.0.txt

@@ -449,10 +449,7 @@ Other Enhancements
 - Integration with the ``feather-format``, including a new top-level ``pd.read_feather()`` and ``DataFrame.to_feather()`` method, see :ref:`here <io.feather>`.
 - ``Series.str.replace()`` now accepts a callable, as replacement, which is passed to ``re.sub`` (:issue:`15055`)
 - ``Series.str.replace()`` now accepts a compiled regular expression as a pattern (:issue:`15446`)
-


@jreback let me know if you want me to revert the deletion of these blank lines.

jreback · 2017-04-14T23:49:02Z

pandas/core/series.py

@@ -2089,21 +2089,33 @@ def map(self, arg, na_action=None):
        two   B
        three C

+        Values in Series that are not in the dictionary (as keys) are converted


put this in Notes section.

jreback · 2017-04-14T23:49:15Z

pandas/core/series.py

+        defines ``__missing__`` (i.e. provides a method for default values),
+        then this default is used rather than ``NaN``:
+
+        >>> from collections import Counter


put this example at the end

jreback · 2017-04-14T23:49:32Z

pandas/tests/series/test_apply.py

+        assert_series_equal(result, expected)
+
+    def test_map_dict_subclass_with_missing(self):
+        class DictWithMissing(dict):


add the issue number here as a comment

jreback · 2017-04-14T23:49:51Z

couple minor doc comments. ping on green.

dhimmel · 2017-04-15T02:42:25Z

>>> "It's \N{green apple}"
"It's 🍏"

@jreback also known as PR is passing all checks

jreback · 2017-04-15T13:46:55Z

thanks!

jreback reviewed Apr 14, 2017

View reviewed changes

dhimmel added 2 commits April 14, 2017 16:03

TST: test series.map on Counter & defaultdict

e8f9d27

series.map: support dicts with defaults

96d12a6

dhimmel force-pushed the map-default-dict branch from a3bcb48 to 96d12a6 Compare April 14, 2017 20:04

Detect default support using __missing__

961ea46

https://docs.python.org/3.6/reference/datamodel.html#object.__missing__

dhimmel force-pushed the map-default-dict branch from 97cfc49 to 961ea46 Compare April 14, 2017 20:28

TST: dict subclasses with/out __missing__

d73cee8

jreback added Enhancement Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate labels Apr 14, 2017

jreback reviewed Apr 14, 2017

View reviewed changes

dhimmel added 3 commits April 14, 2017 17:12

Add v0.20.0 what's new

2a2bab7

DOC: comment series.map

4f3dc6b

DOC: remove unnecessary lambda in formatter

24e1478

jreback reviewed Apr 14, 2017

View reviewed changes

dhimmel added 2 commits April 14, 2017 17:55

DOC: add counter example to docstring

11f5769

DOC: Address review comments

ddb0480

dhimmel changed the title ~~WIP: Support dicts with default values in series.map~~ Support dicts with default values in series.map Apr 14, 2017

dhimmel commented Apr 14, 2017

View reviewed changes

jreback reviewed Apr 14, 2017

View reviewed changes

jreback added this to the 0.20.0 milestone Apr 14, 2017

dhimmel added 2 commits April 14, 2017 20:22

DOC: mention GitHub issue in test docstring

1f56c81

DOC: move missing dict key info to notes

79cfd11

jreback merged commit 61d84db into pandas-dev:master Apr 15, 2017

dhimmel deleted the map-default-dict branch April 15, 2017 23:10

Uh oh!

Support dicts with default values in series.map #16002

Support dicts with default values in series.map #16002

Uh oh!

Conversation

dhimmel commented Apr 14, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback commented Apr 14, 2017

Uh oh!

codecov bot commented Apr 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dhimmel commented Apr 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jreback commented Apr 14, 2017

Uh oh!

dhimmel commented Apr 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jreback commented Apr 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback Apr 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback commented Apr 14, 2017

Uh oh!

dhimmel commented Apr 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback commented Apr 14, 2017

Uh oh!

dhimmel commented Apr 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jreback commented Apr 15, 2017

Uh oh!

Uh oh!

codecov bot commented Apr 14, 2017 •

edited

Loading

dhimmel commented Apr 14, 2017 •

edited

Loading

dhimmel commented Apr 14, 2017 •

edited

Loading

jreback commented Apr 14, 2017 •

edited

Loading

jreback Apr 14, 2017 •

edited

Loading

dhimmel commented Apr 14, 2017 •

edited

Loading

dhimmel commented Apr 15, 2017 •

edited

Loading