[Depr] raise_on_error kwarg with errors kwarg in astype#14878 #14967

m-charlton · 2016-12-22T20:43:29Z

closes DEPR: change .astype(.., raise_on_error) -> errors #14878
tests passed
passes git diff upstream/master | flake8 --diff
whatsnew entry

Please check that the entry in whatsnew/v0.20.0.txt. Unsure that the update was in the
_whatsnew_0200.deprecations or _whatsnew_0200.prior_deprecations so put it in the
former.

…v#14761 Updating documentation to reflect change

DataFrame.astype now allows changing the dtype of a column by passing a dict mapping column name to dtype.

DataFrame.astype now allows setting the type of columns by passing a dict mapping column to dtype.

…com/m-charlton/pandas into update_docs_astype_with_dict_#14761

Mistakenly added changes carried out in v0.19 to v0.20

Valid arguments for new 'errors' kwarg are 'ignore' or 'raise' see pandas-dev#14878

jreback · 2016-12-23T00:08:50Z

pandas/core/generic.py

@@ -3073,7 +3075,9 @@ def astype(self, dtype, copy=True, raise_on_error=True, **kwargs):
            the same type. Alternatively, use {col: dtype, ...}, where col is a
            column label and dtype is a numpy.dtype or Python type to cast one
            or more of the DataFrame's columns to column-specific types.
-        raise_on_error : raise on invalid input


leave this in here and just say DEPRECATED.

@jorisvandenbossche IIRC that is our convention?

add a versionadded tag 0.20.0 for errors

yes, can leave it (but put it at the end)

jreback · 2016-12-23T00:09:08Z

pandas/core/generic.py

-        raise_on_error : raise on invalid input
+        errors : {'raise', 'ignore'}, default 'raise'
+            - ``raise`` : allow exceptions to be raised on invalid input
+            - ``ignore`` : suppress raising exceptions on invalid input
        kwargs : keyword arguments to pass on to the constructor


I think you need a blank line before kwargs (to make the sub-list work)

I've just checked and the sublist is rendered fine with, or without a line between ignore
and kwargs. I can add an extra line if that is the convention,

jreback · 2016-12-23T00:09:59Z

pandas/core/internals.py


-    def _astype(self, dtype, copy=False, raise_on_error=True, values=None,
+    def _astype(self, dtype, copy=False, errors='raise', values=None,
                klass=None, mgr=None, **kwargs):


can you check the errors in ['raise', 'ignore'] at the beginning of the function and raise a ValueError otherwise (and add a test for this)

Sure, there are two 'public' astype(...) methods:

NDFrame.astype(...) in pandas/core/generic.py

Block.astype(...) in pandas/core/internals.py

In addition there is a 'protected' Block._astype(...) method in
pandas/core/internals.py. Should I only put the checks in the 'public'
methods?

Bearing in mind that the raise_on_error kwarg is going to be deprecated for
DataFrame.where() and replaced with the errors kwarg it would make sense to
put the code that checks the validity of the arguments in one place. Do we have
any existing code where we put such validity checking functions/methods?

I notice that both NDFrame & Block inherit from PandasObject but, I'm not
sure that this is the correct thing to do. Should I put a function in
pandas/core/base.py?

Block is completely internal
u can put the check there

we have centralized checks but no need in this case

out in _astype as that's where it's actually used
parameter validation is best down where it's actually

codecov-io · 2016-12-23T00:56:48Z

Current coverage is 84.75% (diff: 100%)

Merging #14967 into master will increase coverage by 0.10%

@@             master     #14967   diff @@
==========================================
  Files           144        145     +1   
  Lines         51030      51144   +114   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43198      43348   +150   
+ Misses         7832       7796    -36   
  Partials          0          0

Powered by Codecov. Last update 8b497e4...0ae8550

…Docstrings to astype clarified.

m-charlton · 2016-12-23T23:20:43Z

Update after review

jorisvandenbossche · 2016-12-23T23:14:38Z

pandas/core/generic.py

+            instead
+        errors : {'raise', 'ignore'}, default 'raise'
+            - ``raise`` : allow exceptions to be raised on invalid input
+            - ``ignore`` : suppress raising exceptions on invalid input


maybe specify what happens of there is an error? (original is returned)

jorisvandenbossche · 2016-12-23T23:17:04Z

pandas/core/internals.py

+
+        if errors not in errors_legal_values:
+            invalid_arg = "Expected value of kwarg 'errors' to be one of %s. "\
+                "Supplied value is '%s'" % (', '.join("'%s'" % arg for arg in


style comment: can you do the string continuation with putting ( ) around it instead of the \. And can you use ´.format(..)´ instead of % (can be on a separate line)

Will do. I'll change the output message to:

Expected value of kwarg 'errors' to be one of ['raise', 'ignore']. Supplied value 'True'

This will do away with the need for the messy formatting.

@m-charlton can you change this to the {} formatting syntax?

jorisvandenbossche · 2016-12-23T23:18:03Z

pandas/tests/test_internals.py

@@ -566,6 +566,13 @@ def test_astype(self):
            else:
                self.assertEqual(tmgr.get('d').dtype.type, t)

+    def test_illegal_arg_for_errors_in_astype(self):
+        """ ValueError exception raised when illegal value used for errors """


Can you turn this into a normal comment (with #)? Otherwise the test name does not show up in the nosetests output.
You can also add the issue number

I would also move the test to the high-level astype tests for frame/series

I've looked at the existing test cases in pandas/tests/series &
pandas/tests/frame and can't find a natural place to put this test.

I was thinking of adding a new file called say test_astype.py in
pandas/tests/frame, as there is an existing test_apply.py

no just grep for astype and you will see lots of tests

ok this test is fine here.

@m-charlton astype tests are located in pandas/tests/frame/test_dtypes.py (you actually updated some tests there)

jreback

small error message formatting change. ping when pushed and green.

jorisvandenbossche · 2017-01-02T08:27:45Z

@m-charlton Can you also add a test confirming that the old keyword still works but gives a deprecation warning?

You can do this with:

        with tm.assert_produces_warning(FutureWarning):
            res = df.astype(.., raise_on_error=False)
        exp = df.astype(.., errors='ignore')

jorisvandenbossche · 2017-01-02T10:22:47Z

xref #14877 we should try to clarify that the errors keyword only is meant for errors during the type conversion. Maybe we can change the "invalid input" to something else? Eg "invalid data for the provided dtype"?

m-charlton · 2017-01-03T10:25:41Z

Back from Christmas/New Year break. Will makes changes today and push for review

Tests added for deprected 'raise_on_error' kwarg & new 'errors' kwarg. Clarified docstring for DataFrame.astype method

jreback · 2017-01-03T13:42:51Z

pandas/tests/frame/test_dtypes.py

@@ -523,6 +523,24 @@ def test_timedeltas(self):
        result = df.get_dtype_counts().sort_values()
        assert_series_equal(result, expected)

+    def test_illegal_arg_for_errors_in_astype(self):


you can make this a single test.

as the same test in tests/series/test_dtypes.py as well

jreback · 2017-01-03T13:43:10Z

minor test change. ping on green.

Unit tests for DataFrame.astype merged. Dupliacted those tests for Series.astype. Both testing deprecation of 'raise_on_error' kwarg.

jorisvandenbossche · 2017-01-03T20:54:08Z

@m-charlton Thanks a lot!

xref pandas-devgh-14967.

xref gh-14967.

xref pandas-devgh-14967.

m-charlton added 14 commits December 9, 2016 11:30

astype method now takes dict mapping col names to datatypes pandas-de…

3f5d728

…v#14761 Updating documentation to reflect change

Updates after review

fddbb2e

DataFrame.astype now allows changing the dtype of a column by passing a dict mapping column name to dtype.

Corrections to docs after review feedback

a61ec51

DataFrame.astype now allows setting the type of columns by passing a dict mapping column to dtype.

Merge branch 'update_docs_astype_with_dict_#14761' of https://github.…

e714d8c

…com/m-charlton/pandas into update_docs_astype_with_dict_#14761

Merge branch 'update_docs_astype_with_dict_#14761'

136175c

Merge remote-tracking branch 'upstream/master'

6282de6

Removing uneeded changes

0c90785

Mistakenly added changes carried out in v0.19 to v0.20

Merge remote-tracking branch 'upstream/master'

bc65ff4

Merge remote-tracking branch 'upstream/master'

cba2b89

Merge remote-tracking branch 'upstream/master'

5626cdd

Merge remote-tracking branch 'upstream/master'

4eb7126

Merge remote-tracking branch 'upstream/master'

6467390

removing stray orig file after merge

f51dbdf

Deprecate 'raise_on_error' kwarg with 'errors' for DataFrame.astype

e2a3f32

Valid arguments for new 'errors' kwarg are 'ignore' or 'raise' see pandas-dev#14878

jreback reviewed Dec 23, 2016

View reviewed changes

jreback added the Deprecate Functionality to remove in pandas label Dec 23, 2016

jreback added this to the 0.20.0 milestone Dec 23, 2016

jsexauer mentioned this pull request Dec 23, 2016

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

Closed

1 task

Check & unit test added for validity of arguments to 'errors' kwarg. …

15bdcf4

…Docstrings to astype clarified.

jorisvandenbossche reviewed Dec 23, 2016

View reviewed changes

jreback requested changes Dec 30, 2016

View reviewed changes

jorisvandenbossche mentioned this pull request Jan 2, 2017

BUG: Respect value of raise_on_error #14877

Closed

Update after code review pandas-dev#1478

39314e6

Tests added for deprected 'raise_on_error' kwarg & new 'errors' kwarg. Clarified docstring for DataFrame.astype method

jreback reviewed Jan 3, 2017

View reviewed changes

Update after code review of unit tests pandas-dev#1478

0ae8550

Unit tests for DataFrame.astype merged. Dupliacted those tests for Series.astype. Both testing deprecation of 'raise_on_error' kwarg.

jreback approved these changes Jan 3, 2017

View reviewed changes

switch kwarg order in docstring

b174e6f

jorisvandenbossche merged commit b957f6f into pandas-dev:master Jan 3, 2017

m-charlton deleted the depr_arg_raise_on_error_in_astype#14878 branch January 4, 2017 10:13

gfyoung added a commit to forking-repos/pandas that referenced this pull request Oct 27, 2018

CLN: Rename raise_on_error to errors for .dtype

234bf2c

xref pandas-devgh-14967.

gfyoung added a commit to forking-repos/pandas that referenced this pull request Oct 27, 2018

CLN: Rename raise_on_error to errors for .dtype

6fb1cec

xref pandas-devgh-14967.

gfyoung mentioned this pull request Oct 27, 2018

CLN: Rename raise_on_error to errors for .dtype #23374

Merged

jreback mentioned this pull request Oct 27, 2018

DEPR: deprecations log for removed issues #13777

Closed

jreback pushed a commit that referenced this pull request Oct 28, 2018

CLN: Rename raise_on_error to errors for .dtype (#23374)

020782a

xref gh-14967.

tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018

CLN: Rename raise_on_error to errors for .dtype (pandas-dev#23374)

4335abd

xref pandas-devgh-14967.

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

CLN: Rename raise_on_error to errors for .dtype (pandas-dev#23374)

7fb40af

xref pandas-devgh-14967.

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

CLN: Rename raise_on_error to errors for .dtype (pandas-dev#23374)

f8f06fa

xref pandas-devgh-14967.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Depr] raise_on_error kwarg with errors kwarg in astype#14878 #14967

[Depr] raise_on_error kwarg with errors kwarg in astype#14878 #14967

m-charlton commented Dec 22, 2016 •

edited by jreback

Loading

jreback Dec 23, 2016

m-charlton Dec 23, 2016

jorisvandenbossche Dec 23, 2016

jreback Dec 23, 2016

m-charlton Dec 23, 2016

jreback Dec 23, 2016

m-charlton Dec 23, 2016

jreback Dec 23, 2016

codecov-io commented Dec 23, 2016 •

edited

Loading

m-charlton commented Dec 23, 2016

jorisvandenbossche Dec 23, 2016

jorisvandenbossche Dec 23, 2016

m-charlton Dec 24, 2016

jreback Dec 30, 2016

m-charlton Jan 3, 2017

jorisvandenbossche Dec 23, 2016

jorisvandenbossche Dec 23, 2016

m-charlton Dec 23, 2016

m-charlton Dec 24, 2016

jreback Dec 24, 2016

jreback Dec 30, 2016

jorisvandenbossche Jan 2, 2017 •

edited

Loading

m-charlton Jan 3, 2017

jreback left a comment

jorisvandenbossche commented Jan 2, 2017

jorisvandenbossche commented Jan 2, 2017

m-charlton commented Jan 3, 2017 •

edited

Loading

jreback Jan 3, 2017

jreback commented Jan 3, 2017

jorisvandenbossche commented Jan 3, 2017

[Depr] raise_on_error kwarg with errors kwarg in astype#14878 #14967

[Depr] raise_on_error kwarg with errors kwarg in astype#14878 #14967

Conversation

m-charlton commented Dec 22, 2016 • edited by jreback Loading

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

codecov-io commented Dec 23, 2016 • edited Loading

Current coverage is 84.75% (diff: 100%)

m-charlton commented Dec 23, 2016

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

Choose a reason for hiding this comment

jorisvandenbossche Jan 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

jorisvandenbossche commented Jan 2, 2017

jorisvandenbossche commented Jan 2, 2017

m-charlton commented Jan 3, 2017 • edited Loading

Choose a reason for hiding this comment

jreback commented Jan 3, 2017

jorisvandenbossche commented Jan 3, 2017

m-charlton commented Dec 22, 2016 •

edited by jreback

Loading

codecov-io commented Dec 23, 2016 •

edited

Loading

jorisvandenbossche Jan 2, 2017 •

edited

Loading

m-charlton commented Jan 3, 2017 •

edited

Loading