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

Add Cython language_level directive to _cantera.pyx #611

Merged
merged 1 commit into from
Mar 11, 2019

Conversation

bryanwweber
Copy link
Member

This fixes the warnings generated by recent versions of Cython that
the language_level will be changed in the future. By setting this
directive, all the code in the .pyx files should be written in
Python 3 syntax. This required several changes to the import
syntax in the files to fix relative vs. absolute imports

See https://stackoverflow.com/questions/54900723/what-does-language-level-in-setup-py-for-cython-do for more information about this setting

This fixes the warnings generated by recent versions of Cython that
the language_level will be changed in the future. By setting this
directive, all the code in the .pyx files should be written in
Python 3 syntax. This required several changes to the import
syntax in the files to fix relative vs. absolute imports
@@ -1,7 +1,7 @@
# This file is part of Cantera. See License.txt in the top-level directory or
# at http://www.cantera.org/license.txt for license and copyright information.

import interrupts
from .interrupts import no_op
Copy link
Member

Choose a reason for hiding this comment

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

The more directly equivalent way to write this would be from . import interrupts, right?

Copy link
Member Author

@bryanwweber bryanwweber Mar 11, 2019

Choose a reason for hiding this comment

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

Yes, I think so, although since its just one function I didn't see the need to keep the namespace. Since that's the only function in that module and that's the only place its used, I thought about removing the module entirely. But since no_op is pure Python, putting it into the .pyx file would cause it to be compiled and expect the type annotations, right?

Copy link
Member

Choose a reason for hiding this comment

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

I guess it's the balance between just one function and the fact that it's only used one time -- the form you used certainly makes sense in the case where the function is used many times.

The reason for this module existing is written in the docstring for the no_op function: "a pure Python function is needed in order for KeyboardInterrupt events to be captured.". I'm not sure if there are any additional considerations regarding type annotations.

Copy link
Member Author

Choose a reason for hiding this comment

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

So if the function were in the .pyx file, it would be compiled and couldn't be used as an interrupt, got it. Thanks!

@speth speth merged commit 6c0866e into Cantera:master Mar 11, 2019
@bryanwweber bryanwweber deleted the cython_language_level branch November 26, 2019 19:07
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.

2 participants