Skip to content

Conversation

sinhrks
Copy link
Member

@sinhrks sinhrks commented Jun 24, 2014

Related to #7485, isnull and notnull is required to be changed.

@@ -237,6 +239,9 @@ cpdef checknull_old(object val):
def isscalar(object val):
return np.isscalar(val) or val is None or PyDateTime_Check(val)

cdef is_periodnat(object val):
from tseries.period import Period
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm...this import is a problem, maybe (hacky), check for attributes freq and ordinal?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can simply cheat hear and do:

return hasattr(val,'ordinal') and val.ordinal == iNaT

and you can make this inline bint as well

@jreback
Copy link
Contributor

jreback commented Jun 24, 2014

see my comments, maybe run a perf check to see if the is_periodnat is an issue

@jreback jreback added Bug and removed Bug labels Jun 24, 2014
@jreback jreback added this to the 0.14.1 milestone Jun 24, 2014
@sinhrks
Copy link
Member Author

sinhrks commented Jun 26, 2014

Thanks. I've ran perf after changing your point, and found some string related methods take 2 times slower than latest commit. Maybe every call of is_periodnat from checknullobj is problematic, I'll consider better approach.

@jreback
Copy link
Contributor

jreback commented Jun 26, 2014

ok, maybe run with those problematic tests again, they are somewhat random (but the import may be a problem).lmk

@jreback
Copy link
Contributor

jreback commented Jul 6, 2014

pushing, if you get too it in next day or 2 pls update

@jreback jreback modified the milestones: 0.15.0, 0.14.1, 0.15.1 Jul 6, 2014
@jreback
Copy link
Contributor

jreback commented Jul 21, 2014

@sinhrks perf on this?

@jreback
Copy link
Contributor

jreback commented Sep 8, 2014

@sinhrks perf on this change?

@jreback
Copy link
Contributor

jreback commented Sep 14, 2014

@sinhrks status?

1 similar comment
@jreback
Copy link
Contributor

jreback commented Sep 26, 2014

@sinhrks status?

@jreback
Copy link
Contributor

jreback commented Sep 29, 2014

@sinhrks ?

@jreback jreback modified the milestones: 0.15.1, 0.15.0 Sep 29, 2014
@jreback
Copy link
Contributor

jreback commented Jan 18, 2015

@sinhrks can you rebase / revisit

@jreback jreback modified the milestones: 0.16.0, Next Major Release Mar 3, 2015
@jreback
Copy link
Contributor

jreback commented May 9, 2015

closing pls reopen if/when updated

@jreback jreback closed this May 9, 2015
@sinhrks
Copy link
Member Author

sinhrks commented May 9, 2015

Yep, this is mostly solved by #9134. There is still an issue in object dtype, but currently I don't have good idea to handle it properly without affecting to performance (and I think below example is not popular case...).

idx = pd.Index(['x', pd.Period('NaT', freq='M')])
pd.isnull(idx)
# [False False]

@sinhrks sinhrks deleted the pnat_isnull branch November 14, 2015 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Period Period data type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants