-
Notifications
You must be signed in to change notification settings - Fork 437
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
Fixed undetected compilation errors prior to Cython 0.22 #598
Conversation
""" Cython version of `Metric.are_compatible`. """ | ||
with gil: | ||
return self.are_compatible(shape2tuple(shape1), shape2tuple(shape2)) | ||
|
||
cdef double c_dist(Metric self, Data2D features1, Data2D features2) nogil: | ||
cdef double c_dist(Metric self, Data2D features1, Data2D features2) nogil except -1.0: |
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.
Does this really need to be a float?
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.
I'm not sure. I think it could be a int
as it will be converted in double
anyway (since the return type of the function is double
).
http://docs.cython.org/src/userguide/language_basics.html#error-return-values
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.
Ok, just tested it, it can be a int
. Do you want me to make the change everywhere?
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.
BTW - ran a try_branch with this:
http://nipy.bic.berkeley.edu/builders/dipy-py2.7-osx-10.8/builds/336
:-)
On Mon, Mar 16, 2015 at 10:21 AM, Marc-Alexandre Côté <
notifications@github.com> wrote:
In dipy/segment/metricspeed.pyx
#598 (comment):""" Cython version of `Metric.are_compatible`. """ with gil: return self.are_compatible(shape2tuple(shape1), shape2tuple(shape2))
- cdef double c_dist(Metric self, Data2D features1, Data2D features2) nogil:
- cdef double c_dist(Metric self, Data2D features1, Data2D features2) nogil except -1.0:
Ok, just tested it, it can be a int. Do you want me to make the change
everywhere?—
Reply to this email directly or view it on GitHub
https://github.com/nipy/dipy/pull/598/files#r26506797.
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.
Yes, please (convert to int). It makes it a bit easier to read, because at the moment I'm thinking 'what is the float for' when the answer is just the usual signal that there is an error.
Good job on the 'try_branch' :)
Travis is not complaining, should be ready to merge. |
Thx @MarcCote. Let's hear what the buildbots say. |
Fixed undetected compilation errors prior to Cython 0.22
Fixes compilation errors in Cython code.
Cython's version 0.22 fixes the following bug "* Mismatching 'except' declarations on signatures in .pxd and .pyx files failed to produce a compile error".
Closes #597