Skip to content

Conversation

jbrockmendel
Copy link
Member

pydatetime_to_datetimestruct does a ton of checking that boils down to "is this a valid datetime object?" Since the function only gets called after a type-check, we can assume it is a date/datetime and be a lot less verbose about it.

This also rips out an unnecessary layer of functions convert_datetime_to_datetimestruct, convert_timedelta_to_timedeltastruct.

cc @WillAyd you mentioned wanting to work on your C-foo. There's a comment about figuring out how to import the cpython datetime C-API. Any thoughts?

@gfyoung gfyoung added Datetime Datetime data dtype Internals Related to non-user accessible pandas implementation Clean labels Jul 18, 2018

cdef inline int64_t pydatetime_to_dt64(datetime val,
npy_datetimestruct *dts):
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

add an explicit check?

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 called by some very perf-sensitive code, all uses of which already handle this carefully. I think its sufficiently internal to be OK without.

@codecov
Copy link

codecov bot commented Jul 18, 2018

Codecov Report

Merging #21962 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #21962   +/-   ##
=======================================
  Coverage   91.96%   91.96%           
=======================================
  Files         166      166           
  Lines       50329    50329           
=======================================
  Hits        46287    46287           
  Misses       4042     4042
Flag Coverage Δ
#multiple 90.36% <ø> (ø) ⬆️
#single 42.23% <ø> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/generic.py 96.46% <0%> (ø) ⬆️

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 4b73f22...69a8459. Read the comment docs.

@jreback jreback added this to the 0.24.0 milestone Jul 20, 2018
@jreback jreback merged commit 1a586e2 into pandas-dev:master Jul 20, 2018
@jreback
Copy link
Contributor

jreback commented Jul 20, 2018

thanks @jbrockmendel

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018
@jbrockmendel jbrockmendel deleted the cless branch April 5, 2020 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Datetime Datetime data dtype Internals Related to non-user accessible pandas implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants