Skip to content

ENH: expose datetime64 conversion functions#16364

Closed
jbrockmendel wants to merge 2 commits intonumpy:mainfrom
jbrockmendel:expose
Closed

ENH: expose datetime64 conversion functions#16364
jbrockmendel wants to merge 2 commits intonumpy:mainfrom
jbrockmendel:expose

Conversation

@jbrockmendel
Copy link
Contributor

These would allow pandas to remove our copy/pasted implementations.

cc @WillAyd @mroeschke anything else we might need?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is sufficient, extension modules aren't allowed to link against each other. This needs to go through the function table like the other NUMPY_API functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, ill see if i can track that down. Before I do, is this likely to be accepted once working?

Copy link
Member

Choose a reason for hiding this comment

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

I think we'd need to think about the function names, but in principle it seems sensible to me to expose the npy_datetimestruct type and some APIs that consume it.

@WillAyd
Copy link
Contributor

WillAyd commented May 27, 2020

For completeness we might want to upstream the timedelta changes as well:

https://github.com/pandas-dev/pandas/blob/707640cc89dd16bff00b62950b3fa27f3b642d5f/pandas/_libs/tslibs/src/datetime/np_datetime_strings.h#L90

Probably in a separate PR, maybe a pre-cursor. The above is used in JSON so I think we wouldn't be able to completely simplify our dependency structure without tacking that as well

@bashtage
Copy link
Contributor

@eric-wieser Not sure why this is tagged random.

@jbrockmendel
Copy link
Contributor Author

Not sure why this is tagged random

@bashtage best guess is that on a previous PR implementing test_cython we adapted the setup from random.tests.test_extending into a fixture and there was an offhand mention of using that pattern in the random tests.

Base automatically changed from master to main March 4, 2021 02:04
@jbrockmendel
Copy link
Contributor Author

Closing in favor of #21199.

jbrockmendel added a commit to jbrockmendel/numpy that referenced this pull request Mar 23, 2022
jbrockmendel added a commit to jbrockmendel/numpy that referenced this pull request Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants