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

Signature error with exceptval #1913

Open
jbrockmendel opened this issue Oct 5, 2017 · 5 comments
Open

Signature error with exceptval #1913

jbrockmendel opened this issue Oct 5, 2017 · 5 comments

Comments

@jbrockmendel
Copy link
Contributor

Just upgraded to 0.27.1, psyched to try out @cython.exceptval. Tried it out on pandas._libs.tslib._Timestamp._compare_outside_nanorange:

Existing:

    cdef bint _compare_outside_nanorange(_Timestamp self, datetime other,
                                         int op) except -1:

New:

    @cython.exceptval(-1)
    @cython.locals(self=_Timestamp, other=datetime, op=int)
    @cython.returns(cython.bint)
    @cython.cfunc
    def _compare_outside_nanorange(self, other, op):

build_ext complained that _compare_outside_nanorange is not declared in the pxd file, so I added it:

cdef class _Timestamp(datetime):
    cdef bint _compare_outside_nanorange(_Timestamp self, datetime other, int op) except -1

build_ext now produces the error message Signature not compatible with previous declaration. AFAICT the signatures should be compatible; am I missing something?

@scoder
Copy link
Contributor

scoder commented Oct 5, 2017 via email

@jbrockmendel
Copy link
Contributor Author

Since you have a .pxd, you don't need the Decorators at all. Well, not in a .py file, at least. Not sure about .pyx.

This is in a pyx. The pxd file contains only the small subset of functions exported. In fact, under the status quo, the _Timestamp class doesn't exist in the pxd at all.

There's no use in typing "self" since its type is obvious.

Makes sense, thanks.

@cython.locals() does not change the signature. That's a known limitation, can't look up the ticket right now.

I'm not sure what to make of this. Elsewhere I've been using (in .pyx files)

@cython.locals(foo=bar)
@cython.cfunc
def somefunc(foo):
    [...]

interchangeably with

cdef somefunc(bar foo):
    [...]

In some of these cases I haven't bothered putting somefunc in the accompanying pxd file, in others it would just be cdef somefunc(bar foo). Have I been using this incorrectly?

@scoder
Copy link
Contributor

scoder commented Oct 7, 2017

May I ask why you seem to prefer the rather verbose decorator syntax over the (IMHO) much more concise dedicated Cython syntax in .pyx files?

If I were to start using a more Python-compatible syntax in .pyx files, I'd certainly prefer the PEP-484 syntax over @locals() and @returns().

@jbrockmendel
Copy link
Contributor Author

A big part of it at this point is just trying to understand what's going on.

Until very recently I was having no luck getting coverage working in non-trivial cython code, so the main motivation was getting pure-python compatibility in order to get coverage statistics. (other tooling e.g. flake8 was/is part of the motivation, but a much lower priority).

I'd certainly prefer the PEP-484

I'm largely working on pandas and statsmodels, which still support py2.

@scoder
Copy link
Contributor

scoder commented Oct 7, 2017

"I'd certainly prefer the PEP-484 syntax"
I'm largely working on pandas and statsmodels, which still support py2.

Question is, do they have to support it uncompiled? If it's enough to serve Py2 with Cython compiled modules, you can happily use Py3 syntax in them (and compile them with language_level=3).

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

No branches or pull requests

2 participants