Skip to content

[CLN] Replace np_datetime code with numpy versions #21852

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

Closed
jbrockmendel opened this issue Jul 11, 2018 · 3 comments
Closed

[CLN] Replace np_datetime code with numpy versions #21852

jbrockmendel opened this issue Jul 11, 2018 · 3 comments
Labels
Clean Dependencies Required and optional dependencies

Comments

@jbrockmendel
Copy link
Member

@jreback we discussed this briefly, hopefully you'll have better luck with it than me.

pandas/_libs/src/datetime/np_datetime.c and np_datetime_strings.c share a ton of code with numpy:

https://github.com/numpy/numpy/blob/master/numpy/core/src/multiarray/datetime.c
https://github.com/numpy/numpy/blob/master/numpy/core/src/multiarray/_datetime.h
https://github.com/numpy/numpy/blob/master/numpy/core/src/multiarray/datetime_strings.c
https://github.com/numpy/numpy/blob/master/numpy/core/src/multiarray/datetime_strings.h

My attempts so far to "get at" these numpy functions via #include have failed, someone with stronger C-fu than me will have to try. The testing ground I've been trying on is src/ujson/python/objToJSON.c since all of the np_datetime functions it uses have drop-in counterparts in numpy:

PANDAS_DATETIMEUNIT --> NPY_DATETIMEUNIT
PANDAS_FR_ns --> NPY_FR_ns (ditto s, ms, us)
pandas_datetimestruct --> npy_datetimestruct

get_datetime_iso_8601_strlen --> identical

make_iso_8601_datetime has fewer args in the pandas version, largely since we removed unnecessary ones a few months ago. Adding hard-coded args in should make them equivalent.

convert_pydatetime_to_datetimestruct also looks like dummy arguments need to be re-introduced to make them identical.

convert_datetime_to_datetimestruct has a different first-argument (again one we simplified) but is functionally identical.
convert_datetimestruct_to_datetime ditto.

parse_iso_8601_datetime has non-trivial differences IIRC.

@jbrockmendel jbrockmendel added Clean Dependencies Required and optional dependencies labels Jul 11, 2018
@jbrockmendel
Copy link
Member Author

Is the conclusion from #21863 that this is either infeasible or undesirable?

@jreback
Copy link
Contributor

jreback commented Jul 12, 2018

no this is possible ; just wanted to see if we could document the original reasons for using vendors approaches

@jbrockmendel
Copy link
Member Author

This is not actionable until numpy/numpy#9675 is addressed there. I'll keep an eye on that; closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Dependencies Required and optional dependencies
Projects
None yet
Development

No branches or pull requests

2 participants