-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
NaT inheritance #17435
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
Comments
Hmm...this is an interesting proposal. Being able to disentangle the mess of Cython code beneath |
In case anyone is curious where I'm going with the whole Once the current set of PRs is resolved, I'll make one to separate out Once
|
OK, we've gotten to a point in The proposed module That said, it is not a matter of cut/paste the same way as other
Under the proposed change, these
@jreback I'd like to get your feedback on this when convenient; no hurry. |
pd.NaT
is an instance ofNaTType
.NaTType
inherits from_NaT
, which inherits from_Timestamp
, which inherits fromdatetime
.How would people feel about taking
_Timestamp
out of that chain? Note that_Timestamp
is not exported, so no users should be checking e.g.isinstance(x, _Timestamp)
.The motivation: if we can take
_Timestamp
out of the inheritance chain, then the entireNaT
hierarchy along with the instance itself can be defined in its own dedicated module, with no intra-pandas dependencies other thanutil
. This comes out to a little over 500 lines.This opens up opportunities for simplification elsewhere, since there are a bunch of modules that import
NaT
but don't care about most of the rest oftslib
(the same applies to the timezones functions in #17274). Of particular interest are_libs.lib
,_libs.period
,tseries.frequencies
, andtseries.offsets
.The other upside is that with few dependencies, the module becomes much easier to test/benchmark/maintain.
Without
_NaT
inheriting from_Timestamp
, a methods in_Timestamp
become eligible for the@cython.final
decorator. My (limited) understanding is that this provides a small performance improvement.The main downside I see is that the docstring pinning done by
_make_error_func
,_make_nat_func
, and_make_nan_func
is a bit of a hassle.The text was updated successfully, but these errors were encountered: