-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
pythonize cython code #22638
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
pythonize cython code #22638
Conversation
@@ -374,7 +374,7 @@ cpdef bint is_subperiod(source, target): | |||
return source in {'N'} | |||
|
|||
|
|||
cpdef bint is_superperiod(source, target): | |||
def is_superperiod(source, target) -> bint: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two are not used within cython, I think they were cpdef just because with the other syntax you can’t declare a return type with just def.
Codecov Report
@@ Coverage Diff @@
## master #22638 +/- ##
=======================================
Coverage 92.17% 92.17%
=======================================
Files 169 169
Lines 50708 50708
=======================================
Hits 46740 46740
Misses 3968 3968
Continue to review full report at Codecov.
|
thanks. maybe add a note somewhere that we can use type annotation in cython files (maybe at the top of each file?) |
Sorry, why do we think this is an improvement? Now we have two ways of doing things? |
Some discussion in #22180. Main motivation: I think that if we can get the code close enough to valid python, we can trick flake8 into running on it. |
Use python-style type annotations instead of cython-style in a few places.
Use python-style isinstance checks in cases where cython will automatically optimize them into C calls.
Part of the hope is that we can get the code close enough to valid python that we can trick flake8 into working on it.