CLN: core/dtypes/cast.py::maybe_downcast_to_dtype #37050

arw2019 · 2020-10-11T04:17:50Z

jbrockmendel · 2020-10-11T14:53:09Z

pandas/core/dtypes/cast.py

-    if dtype.kind in ["M", "m"] and result.dtype.kind in ["i", "f"]:
-        if hasattr(dtype, "tz"):
+    # GH12821, iNaT is cast to float
+    if is_datetime_or_timedelta_any_dtype(dtype) and result.dtype.kind in ["i", "f"]:


the dtype.kind checks are much more performant

i agree, just leave this as is i think for L163 (and can remove the comment which isold)

Ok! Done both

jreback · 2020-10-11T16:08:13Z

pandas/core/dtypes/cast.py

-    if dtype.kind in ["M", "m"] and result.dtype.kind in ["i", "f"]:
-        if hasattr(dtype, "tz"):
+    # GH12821, iNaT is cast to float
+    if is_datetime_or_timedelta_any_dtype(dtype) and result.dtype.kind in ["i", "f"]:


i agree, just leave this as is i think for L163 (and can remove the comment which isold)

jreback · 2020-10-11T16:35:52Z

pandas/core/dtypes/cast.py

    if dtype.kind in ["M", "m"] and result.dtype.kind in ["i", "f"]:
-        if hasattr(dtype, "tz"):
+
+        if is_datetime_or_timedelta_dtype(dtype):


i think this code here can be greatly simplified, see what the incoming inputs are and i bet you can adjust it

IOW the way this is written L173 is always hit

tried this. There are three tests that use the else block:

FAILED pandas/tests/dtypes/cast/test_downcast.py::test_datetime_with_timezone[True] FAILED pandas/tests/frame/methods/test_combine_first.py::TestDataFrameCombineFirst::test_combine_first_timezone FAILED pandas/tests/groupby/test_function.py::test_arg_passthru - TypeError: ...

Reverting

and you have L177. so i would revert to the original code here as well (except for the period change);

jreback · 2020-10-12T15:27:47Z

pandas/core/dtypes/cast.py

    if dtype.kind in ["M", "m"] and result.dtype.kind in ["i", "f"]:
-        if hasattr(dtype, "tz"):
+
+        if is_datetime_or_timedelta_dtype(dtype):


and you have L177. so i would revert to the original code here as well (except for the period change);

jreback · 2020-10-14T12:32:47Z

thanks @arw2019

simonjayhawkins · 2020-10-14T15:42:00Z

@arw2019 this solution could be to revert this and re-open? The mypy errors are

pandas\core\dtypes\cast.py:173: error: Item "type" of "Union[ExtensionDtype, Any, Type[object]]" has no attribute "type"  [union-attr]
pandas\core\dtypes\cast.py:179: error: Item "ExtensionDtype" of "Union[ExtensionDtype, Any, Type[object]]" has no attribute "freq"  [union-attr]
pandas\core\dtypes\cast.py:179: error: Item "type" of "Union[ExtensionDtype, Any, Type[object]]" has no attribute "freq"  [union-attr]

so i'm a bit reluctant to ignore to get mypy to green.

The issue is related to #37024 (comment)

so maybe only str and dtypeObj are accepted. so could also change to Union[str, DtypeObj] but this I think is unusual and should normally be DtypeObj or Dtype.

This reverts commit fad14fd.

arw2019 · 2020-10-14T16:01:17Z

@arw2019 this solution could be to revert this and re-open? The mypy errors are

pandas\core\dtypes\cast.py:173: error: Item "type" of "Union[ExtensionDtype, Any, Type[object]]" has no attribute "type"  [union-attr]
pandas\core\dtypes\cast.py:179: error: Item "ExtensionDtype" of "Union[ExtensionDtype, Any, Type[object]]" has no attribute "freq"  [union-attr]
pandas\core\dtypes\cast.py:179: error: Item "type" of "Union[ExtensionDtype, Any, Type[object]]" has no attribute "freq"  [union-attr]

so i'm a bit reluctant to ignore to get mypy to green.

Your call since I'm still pretty new to the code base.

The issue is related to #37024 (comment)

so maybe only str and dtypeObj are accepted. so could also change to Union[str, DtypeObj] but this I think is unusual and should normally be DtypeObj or Dtype.

Again I'm fairly new but I think it's possible Union[str, DtypeObj] is right, at least based on the tests we've got for this:

(pandas-dev) root@27041e356f25:/workspaces/pandas-arw2019# grep -r -n maybe_downcast_to_dtype pandas/tests
pandas/tests/dtypes/cast/test_downcast.py:6:from pandas.core.dtypes.cast import maybe_downcast_to_dtype
pandas/tests/dtypes/cast/test_downcast.py:40:    result = maybe_downcast_to_dtype(arr, dtype)
pandas/tests/dtypes/cast/test_downcast.py:47:    result = maybe_downcast_to_dtype(ser, np.dtype(np.float64))
pandas/tests/dtypes/cast/test_downcast.py:58:    result = maybe_downcast_to_dtype(arr, "infer")
pandas/tests/dtypes/cast/test_downcast.py:69:    result = maybe_downcast_to_dtype(arr, "infer")
pandas/tests/dtypes/cast/test_downcast.py:76:    result = maybe_downcast_to_dtype(arr, "int64")
pandas/tests/dtypes/cast/test_downcast.py:86:    res = maybe_downcast_to_dtype(arr, dtype)
pandas/tests/dtypes/cast/test_downcast.py:97:    res = maybe_downcast_to_dtype(obj, exp.dtype)

For reference actually only use this method in a few places in the code:

pandas/core/frame.py:88:    maybe_downcast_to_dtype,
pandas/core/frame.py:6247:            arr = maybe_downcast_to_dtype(arr, this_dtype)
pandas/core/indexes/interval.py:22:    maybe_downcast_to_dtype,
pandas/core/indexes/interval.py:1301:            breaks = maybe_downcast_to_dtype(breaks, "int64")
pandas/core/tools/numeric.py:5:from pandas.core.dtypes.cast import maybe_downcast_to_dtype
pandas/core/tools/numeric.py:182:                    values = maybe_downcast_to_dtype(values, dtype)
pandas/core/reshape/pivot.py:18:from pandas.core.dtypes.cast import maybe_downcast_to_dtype
pandas/core/reshape/pivot.py:128:                agged[v] = maybe_downcast_to_dtype(agged[v], data[v].dtype)
pandas/core/reshape/pivot.py:275:                maybe_downcast_to_dtype, args=(dtype,)
pandas/core/dtypes/cast.py:122:def maybe_downcast_to_dtype(result, dtype):
pandas/core/dtypes/cast.py:189:    Subset of maybe_downcast_to_dtype restricted to numeric dtypes.
pandas/core/dtypes/cast.py:298:            result = maybe_downcast_to_dtype(result, dtype)
pandas/core/internals/blocks.py:24:    maybe_downcast_to_dtype,
pandas/core/internals/blocks.py:533:            nv = maybe_downcast_to_dtype(values, dtypes)
pandas/core/internals/blocks.py:550:            val = maybe_downcast_to_dtype(val, dtype="infer")

simonjayhawkins · 2020-10-14T16:10:35Z

we'll go with the revert for now even though the fix is straightfoward to allow unpressured discussion on the topic. OK with reopening this PR?

arw2019 · 2020-10-14T16:15:11Z

we'll go with the revert for now even though the fix is straightfoward to allow unpressured discussion on the topic. OK with reopening this PR?

Sounds good to me!

…37116) This reverts commit fad14fd.

…#37050)" (pandas-dev#37116) This reverts commit fad14fd.

arw2019 marked this pull request as draft October 11, 2020 04:24

arw2019 marked this pull request as ready for review October 11, 2020 05:47

arw2019 changed the title ~~[WIP] CLN: core/dtypes/cast.py::maybe_downcast_to_dtype~~ CLN: core/dtypes/cast.py::maybe_downcast_to_dtype Oct 11, 2020

jbrockmendel reviewed Oct 11, 2020

View reviewed changes

jreback added Clean Dtype Conversions Unexpected or buggy dtype conversions labels Oct 11, 2020

jreback requested changes Oct 11, 2020

View reviewed changes

arw2019 force-pushed the maybe_downcast_to_dtype_cleanup branch from a5c9169 to 3aad776 Compare October 11, 2020 16:23

jreback reviewed Oct 11, 2020

View reviewed changes

jreback added this to the 1.2 milestone Oct 12, 2020

jreback requested changes Oct 12, 2020

View reviewed changes

arw2019 added 10 commits October 12, 2020 17:06

CLN: rewrite try/except/pass using contextlib.suppress

a8eba60

use is_period_dtype to check type

058bf6f

clean up if condition

9eb0b67

add is_datetime_or_timedelta_any_dtype helper to dtypes/common

c2ad3d9

enact TODO: relocate PeriodArray block

9c06bc6

feedback: revert changes to dtype checks

0a23240

reorder clause

e2d0375

feedback: collapse datetimelike if body

b5e53e9

revert datelike if-else body

d887566

feedback: revert datetimelike block

e46b10d

arw2019 force-pushed the maybe_downcast_to_dtype_cleanup branch from 51557f4 to e46b10d Compare October 12, 2020 17:06

jreback approved these changes Oct 14, 2020

View reviewed changes

jreback merged commit fad14fd into pandas-dev:master Oct 14, 2020

simonjayhawkins mentioned this pull request Oct 14, 2020

VIS: Accept xlabel and ylabel for scatter and hexbin plots #37102

Merged

5 tasks

simonjayhawkins mentioned this pull request Oct 14, 2020

Revert "CLN: core/dtypes/cast.py::maybe_downcast_to_dtype" #37116

Merged

simonjayhawkins added a commit that referenced this pull request Oct 14, 2020

Revert "CLN: core/dtypes/cast.py::maybe_downcast_to_dtype (#37050)"

47c933a

This reverts commit fad14fd.

simonjayhawkins added a commit that referenced this pull request Oct 14, 2020

Revert "CLN: core/dtypes/cast.py::maybe_downcast_to_dtype (#37050)" (#…

c0c3516

…37116) This reverts commit fad14fd.

arw2019 mentioned this pull request Oct 15, 2020

[REDO] CLN: core/dtypes/cast.py::maybe_downcast_to_dtype #37126

Merged

5 tasks

JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Oct 26, 2020

CLN: core/dtypes/cast.py::maybe_downcast_to_dtype (pandas-dev#37050)

e428559

JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Oct 26, 2020

Revert "CLN: core/dtypes/cast.py::maybe_downcast_to_dtype (pandas-dev…

fee0afa

…#37050)" (pandas-dev#37116) This reverts commit fad14fd.

kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020

CLN: core/dtypes/cast.py::maybe_downcast_to_dtype (pandas-dev#37050)

fd055ca

kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020

Revert "CLN: core/dtypes/cast.py::maybe_downcast_to_dtype (pandas-dev…

33d7b8d

…#37050)" (pandas-dev#37116) This reverts commit fad14fd.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLN: core/dtypes/cast.py::maybe_downcast_to_dtype #37050

CLN: core/dtypes/cast.py::maybe_downcast_to_dtype #37050

arw2019 commented Oct 11, 2020

jbrockmendel Oct 11, 2020

jreback Oct 11, 2020

arw2019 Oct 11, 2020

jreback Oct 11, 2020

jreback Oct 11, 2020

jreback Oct 11, 2020

arw2019 Oct 11, 2020

jreback Oct 12, 2020

arw2019 Oct 12, 2020

jreback Oct 12, 2020

jreback commented Oct 14, 2020

simonjayhawkins commented Oct 14, 2020

arw2019 commented Oct 14, 2020

simonjayhawkins commented Oct 14, 2020

arw2019 commented Oct 14, 2020

CLN: core/dtypes/cast.py::maybe_downcast_to_dtype #37050

CLN: core/dtypes/cast.py::maybe_downcast_to_dtype #37050

Conversation

arw2019 commented Oct 11, 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

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

simonjayhawkins commented Oct 14, 2020

arw2019 commented Oct 14, 2020

simonjayhawkins commented Oct 14, 2020

arw2019 commented Oct 14, 2020