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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions interfaces/cython/cantera/_cantera.pxd
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# 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.
# cython: language_level=3

from libcpp.vector cimport vector
from libcpp.string cimport string
Expand Down
2 changes: 1 addition & 1 deletion interfaces/cython/cantera/_cantera.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import math

from cython.operator cimport dereference as deref, preincrement as inc

from _cantera cimport *
from ._cantera cimport *

include "utils.pyx"
include "constants.pyx"
Expand Down
4 changes: 2 additions & 2 deletions interfaces/cython/cantera/onedim.pyx
Original file line number Diff line number Diff line change
@@ -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!

import warnings

# Need a pure-python class to store weakrefs to
Expand Down Expand Up @@ -589,7 +589,7 @@ cdef class Sim1D:

self.sim = new CxxSim1D(D)
self.domains = tuple(domains)
self.set_interrupt(interrupts.no_op)
self.set_interrupt(no_op)
self._initialized = False
self._initial_guess_args = ()
self._initial_guess_kwargs = {}
Expand Down
2 changes: 1 addition & 1 deletion interfaces/cython/cantera/reactionpath.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ cdef class ReactionPathDiagram:
self.diagram, True)
self.built = True
if verbose:
print self.log
print(self.log)

property log:
"""
Expand Down