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

CLN: Datetimelike._can_hold_na #13983

Merged
merged 1 commit into from
Sep 1, 2016

Conversation

sinhrks
Copy link
Member

@sinhrks sinhrks commented Aug 13, 2016

  • tests added / passed
  • passes git diff upstream/master | flake8 --diff
  • no whatsnew, as it is internal fix

datetimelike Index._can_hold_na is all False even though these can store NaT.

idx = pd.DatetimeIndex(['2011-01-01', 'NaT'])
idx.hasnans
# True

idx._can_hold_na
# False

idx._nan_idxs
# array([], dtype=int64)

Because of test_base.py condition below, datetime-like Indexes had skipped some tests until now.

@sinhrks sinhrks added Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Clean labels Aug 13, 2016
@sinhrks sinhrks added this to the 0.19.0 milestone Aug 13, 2016
@codecov-io
Copy link

codecov-io commented Aug 13, 2016

Current coverage is 85.28% (diff: 100%)

Merging #13983 into master will increase coverage by <.01%

@@             master     #13983   diff @@
==========================================
  Files           139        139          
  Lines         50543      50547     +4   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43101      43107     +6   
+ Misses         7442       7440     -2   
  Partials          0          0          

Powered by Codecov. Last update 8fdfa51...5435247

@sinhrks sinhrks force-pushed the can_hold_na branch 4 times, most recently from ec38c18 to dbdac92 Compare August 16, 2016 14:08
@sinhrks sinhrks changed the title (WIP) CLN: Datetimelike._can_hold_na CLN: Datetimelike._can_hold_na Aug 16, 2016
@sinhrks sinhrks force-pushed the can_hold_na branch 3 times, most recently from 70a4e82 to da4e886 Compare August 16, 2016 21:18
@sinhrks
Copy link
Member Author

sinhrks commented Aug 16, 2016

Updated not to rely on #13979. _get_unique_index can be removed after #13984.

@sinhrks
Copy link
Member Author

sinhrks commented Aug 21, 2016

@jreback @jorisvandenbossche can u check? This is needed to make some Index dtype related fixes.

@sinhrks sinhrks force-pushed the can_hold_na branch 2 times, most recently from 7e60ff5 to fb23295 Compare August 26, 2016 23:06
@jorisvandenbossche
Copy link
Member

This is looking good to me. @jreback OK?

@jreback
Copy link
Contributor

jreback commented Aug 27, 2016

needs #13979 first

"""
wrap Index._get_unique_index to handle NaT
"""
res = super(PeriodIndex, self)._get_unique_index(dropna=dropna)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be handled in the super?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed to handle NaT. #13979 and #13984 makes _get_unique_index itself unnecessary.

@jorisvandenbossche
Copy link
Member

@sinhrks updated to not rely on that one

@sinhrks
Copy link
Member Author

sinhrks commented Aug 29, 2016

@jreback Now this PR doesn't rely on #13979.

@jorisvandenbossche
Copy link
Member

@sinhrks This needs a rebase now I merged the other

@jreback
Copy link
Contributor

jreback commented Aug 31, 2016

lgtm. ping on green.

@sinhrks
Copy link
Member Author

sinhrks commented Sep 1, 2016

fixed lint error, and now green.

@jorisvandenbossche jorisvandenbossche merged commit 70bb179 into pandas-dev:master Sep 1, 2016
@jorisvandenbossche
Copy link
Member

Thanks!

@sinhrks sinhrks deleted the can_hold_na branch September 3, 2016 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants