Skip to content

POC: make ccalendar.pyx valid python #35054

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

Conversation

jbrockmendel
Copy link
Member

Proof of concept for making some of our cython files into valid python*, which would allow us to lint and maybe (cc @simonjayhawkins thoughts?) use mypy

* strictly speaking, we need to do a find-and-replace of "cimport" -> "import" to get all the way to valid python.

@simonjayhawkins
Copy link
Member

cool. i've been looking into this but not got very far since I need to better understand cython first.

One thing that I have thought while investigating is that it may be possible to move some non performance related code (not looping) out of the cython.

for instance in array_with_unit_to_datetime in pandas/_libs/tslib.pyx, there seems to be logic to determine whether to iterate. I wonder whether code like this could be in python so that the cython code is just the iteration. (I think some more code that could be pure python being added in #35027?)

of course the approach in this PR helps us to keep relevant code together, while still allowing linting and mypy checking.

@WillAyd
Copy link
Member

WillAyd commented Jun 29, 2020

Is this style documented by Cython?

@jbrockmendel
Copy link
Member Author

Is this style documented by Cython?

It's documented as "pure-python mode".

One thing that I have thought while investigating is that it may be possible to move some non performance related code (not looping) out of the cython.

As long as it doesn't break the self-contained-ness of tslibs and near-self-contained-ness of _libs, I'm all for this.

@simonjayhawkins is this something where we get a non-trivial benefit even if we only get a handful of modules? This is one of the easiest modules to do this on. In particular pure-python mode doesn't support const int64_t[:] or ndarray[int64_t] etc.

@jbrockmendel
Copy link
Member Author

Closing to clear the queue since this is just a proof of concept. Can re-open if @simonjayhawkins thinks there is benefit to getting some but not all of these files python-valid

@jbrockmendel jbrockmendel deleted the ref-tslibs-valid-python branch November 20, 2021 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants