Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: repr of np.datetime64('NaT') in Series/DataFrame with dtype object #25445

Merged
merged 10 commits into from Mar 30, 2019

Conversation

@mroeschke
Copy link
Member

commented Feb 26, 2019

  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

Discovered while investigating #12499

Before:

In [1]: pd.Series([np.datetime64('NaT')], dtype=object)
Out[1]:
0    NaN
dtype: object

After:

In [1]: pd.Series([np.datetime64('NaT')], dtype=object)
Out[1]:
0    NaT
dtype: object
Matt Roeschke
@codecov

This comment has been minimized.

Copy link

commented Feb 26, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #25445      +/-   ##
==========================================
+ Coverage   91.74%   91.74%   +<.01%     
==========================================
  Files         173      173              
  Lines       52923    52924       +1     
==========================================
+ Hits        48554    48556       +2     
+ Misses       4369     4368       -1
Flag Coverage Δ
#multiple 90.31% <100%> (ø) ⬆️
#single 41.73% <50%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/io/formats/format.py 97.99% <100%> (ø) ⬆️
pandas/util/testing.py 87.66% <0%> (+0.09%) ⬆️

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 aa08416...b7aaa02. Read the comment docs.

@codecov

This comment has been minimized.

Copy link

commented Feb 26, 2019

Codecov Report

Merging #25445 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #25445      +/-   ##
==========================================
+ Coverage   91.77%   91.79%   +0.01%     
==========================================
  Files         175      174       -1     
  Lines       52606    52539      -67     
==========================================
- Hits        48280    48229      -51     
+ Misses       4326     4310      -16
Flag Coverage Δ
#multiple 90.35% <100%> (+0.02%) ⬆️
#single 41.88% <75%> (-0.11%) ⬇️
Impacted Files Coverage Δ
pandas/io/formats/format.py 97.88% <100%> (-0.11%) ⬇️
pandas/io/gbq.py 75% <0%> (-12.5%) ⬇️
pandas/core/config_init.py 96.96% <0%> (-2.24%) ⬇️
pandas/core/computation/common.py 89.47% <0%> (-0.53%) ⬇️
pandas/compat/pickle_compat.py 69.13% <0%> (-0.38%) ⬇️
pandas/compat/numpy/__init__.py 92.85% <0%> (-0.25%) ⬇️
pandas/plotting/_style.py 77.17% <0%> (-0.25%) ⬇️
pandas/core/computation/engines.py 88.52% <0%> (-0.19%) ⬇️
pandas/plotting/_timeseries.py 65.28% <0%> (-0.18%) ⬇️
pandas/io/excel/_util.py 87.5% <0%> (-0.18%) ⬇️
... and 40 more

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 7721f70...8d8abd1. Read the comment docs.

@@ -946,7 +947,7 @@ def _format(x):
if self.na_rep is not None and is_scalar(x) and isna(x):
if x is None:
return 'None'
elif x is NaT:
elif x is NaT or is_np_nat(x):

This comment has been minimized.

Copy link
@jreback

jreback Feb 27, 2019

Contributor

use is_null_datetimelike instead of rolling your own

This comment has been minimized.

Copy link
@mroeschke

mroeschke Feb 28, 2019

Author Member

is_null_datetimelike is too permissive here (accepts np.nan and None)

This comment has been minimized.

Copy link
@jreback

jreback Feb 28, 2019

Contributor

I don't want yet another NaT detection routine.

This comment has been minimized.

Copy link
@mroeschke

mroeschke Mar 1, 2019

Author Member

is_null_datetimelike doesn't just detect NaT though. It checks if any args are None, np.nan, NaT, iNaT

@jreback
Copy link
Contributor

left a comment

see comment

@mroeschke mroeschke force-pushed the mroeschke:format_np_nat_object branch from cf7f51a to b7aaa02 Feb 28, 2019

@mroeschke mroeschke added this to the 0.25.0 milestone Feb 28, 2019

@@ -946,7 +947,7 @@ def _format(x):
if self.na_rep is not None and is_scalar(x) and isna(x):
if x is None:
return 'None'
elif x is NaT:
elif x is NaT or is_np_nat(x):

This comment has been minimized.

Copy link
@jreback

jreback Feb 28, 2019

Contributor

I don't want yet another NaT detection routine.

@jreback

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

let's wait till we merge the min numpy 1.13 and then revist

@@ -14,6 +14,7 @@
from pandas._libs import lib
from pandas._libs.tslib import format_array_from_datetime
from pandas._libs.tslibs import NaT, Timedelta, Timestamp, iNaT
from pandas._libs.tslibs.nattype import is_np_nat

This comment has been minimized.

Copy link
@jreback

jreback Mar 29, 2019

Contributor

this needs to be removed

return 'None'
elif x is NaT:
return 'NaT'
try:

This comment has been minimized.

Copy link
@jreback

jreback Mar 29, 2019

Contributor

can you add a comment here on what is going on here

[Series, '0 NaT\ndtype: object'],
[DataFrame, ' 0\n0 NaT']])
def test_repr_np_nat_with_object(self, arg, box, expected):
result = repr(box([arg('NaT')], dtype=object))

This comment has been minimized.

Copy link
@jreback

jreback Mar 29, 2019

Contributor

can you add the issue number

Matt Roeschke
@mroeschke

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2019

Address comments and all green

@jreback jreback merged commit cb89bc0 into pandas-dev:master Mar 30, 2019

11 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
pandas-dev.pandas Build #20190329.33 succeeded
Details
pandas-dev.pandas (Checks_and_doc) Checks_and_doc succeeded
Details
pandas-dev.pandas (Linux py35_compat) Linux py35_compat succeeded
Details
pandas-dev.pandas (Linux py36_locale_slow) Linux py36_locale_slow succeeded
Details
pandas-dev.pandas (Linux py36_locale_slow_old_np) Linux py36_locale_slow_old_np succeeded
Details
pandas-dev.pandas (Linux py37_locale) Linux py37_locale succeeded
Details
pandas-dev.pandas (Linux py37_np_dev) Linux py37_np_dev succeeded
Details
pandas-dev.pandas (Windows py36_np15) Windows py36_np15 succeeded
Details
pandas-dev.pandas (Windows py37_np141) Windows py37_np141 succeeded
Details
pandas-dev.pandas (macOS py35_macos) macOS py35_macos succeeded
Details
@jreback

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2019

thanks @mroeschke

@mroeschke mroeschke deleted the mroeschke:format_np_nat_object branch Mar 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.