Skip to content
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

[DISC] Use type annotations in cython #22180

Closed
jbrockmendel opened this issue Aug 2, 2018 · 3 comments
Closed

[DISC] Use type annotations in cython #22180

jbrockmendel opened this issue Aug 2, 2018 · 3 comments
Labels
Docs Internals Related to non-user accessible pandas implementation Needs Discussion Requires discussion from core team before further action

Comments

@jbrockmendel
Copy link
Member

Regardless of whether run in py2/py3, cython supports modern type annotations. A lot of _libs could be made valid python by using these.

Upsides:

  • linting!
  • get experience with these, will help in deciding if we want to use them in the non-cython code-base.
  • may lower barrier-to-entry for contributors not comfortable with cython

Downsides:

  • If it works, don't fix it
  • Until fully implemented, we'd have mix-and-match conventions
  • cython's pure-python mode has more rough edges than its regular mode
  • in some cases the valid-python versions get pretty verbose:
cdef inline bint is_integer_object(object obj) nogil:
    cdef:
         int foo

vs.

@cython.cfunc
@cython.inline
@cython.nogil
def is_integer_object(tz: object) -> bint:
    foo: int

The main upside I see is the linting. My inclination is to try to implement it in a subset of files where we can get close to pure-python as a test run.

Thoughts?

@WillAyd
Copy link
Member

WillAyd commented Aug 2, 2018

Is this a new feature? If it's as advertised I'm +1 as it would probably even make it easier for newcomers to understand the Cython code base

@gfyoung gfyoung added Internals Related to non-user accessible pandas implementation Docs labels Aug 3, 2018
@gfyoung
Copy link
Member

gfyoung commented Aug 3, 2018

+1 from me as well.

@jbrockmendel jbrockmendel added the Needs Discussion Requires discussion from core team before further action label Aug 5, 2018
@jbrockmendel
Copy link
Member Author

Opened cython#2529 to discuss some syntax combinations that are parsed incorrectly (or not supported, but not made explicit in the docs). I think this is worth revisiting a couple of cython releases down the road, but not just yet. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Internals Related to non-user accessible pandas implementation Needs Discussion Requires discussion from core team before further action
Projects
None yet
Development

No branches or pull requests

3 participants