-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
REF: avoid importing pd.NA in tslibs #34009
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
Conversation
This keeps importing pd.NA into tslibs, though? (but in tslibs.pyx itself it is less a problem?) The behaviour change is certainly fine for me. I think it is even better than we raise for pd.NA for now (since pd.NA is not yet used in any datetimelike dtype. |
tslib.pyx, but not the tslibs directory, right. Since nothing inside tslibs/ depends on tslib.pyx, it is not that big a deal. |
Ah, I missed the tslib.pyx vs tslib directory |
can you merge master. do we have sufficient test for this change? |
We could in principle test |
The existing cimport of pd.NA into tslibs.nattype is a circular dependency (its not actually clear to me why it doesnt raise). I'm finding that its presence is making other refactoring around tslibs.offsets more difficult. This PR moves the import of pd.NA to tslib, breaking the circular dependency.
This does change some behavior, but none that is tested, so we should discuss what places we want to recognize pd.NA. In particular,
pd.Timedelta(pd.NA)
returnsNaT
in master and raisesValueError
in this PR.pd.Timestamp(pd.NA)
andpd.Period(pd.NA)
raise TypeError and ValueError, respectively, in both master and this PR.cc @jorisvandenbossche what is your preferred behavior?