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

Variable naming seems to be taken from .pxd in Cython 0.27.0 #1888

Open
honnibal opened this Issue Sep 26, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@honnibal

honnibal commented Sep 26, 2017

I had a function where the parameter name was different in the .pyx and the .pxd, like this:

# .pxd
cdef int do_stuff(int foo) except -1

# .pyx
cdef int do_stuff(int bar) except -1:
   return bar

I didn't try exactly this code -- the real example can be seen here: https://github.com/explosion/spaCy/blob/2480f8f521ea22b74ff90aed0853e9c0e2543b1b/spacy/tokens/doc.pyx#L494

This code compiled under Cython <0.27.0 (although I'm uncertain about whether it should have).
In Cython 0.27.0, I get a name error saying bar is not defined. (In my actual example, it was has_space). It seems that the variable name is being drawn from the .pxd declaration?

@scoder

This comment has been minimized.

Show comment
Hide comment
@scoder

scoder Sep 26, 2017

Contributor
Contributor

scoder commented Sep 26, 2017

scoder added a commit that referenced this issue Sep 26, 2017

Add test for ticket #1888: mismatching argument names between .pxd an…
…d .pyx files are (not great but) actually ok, but the callers must always see the "public" pxd names, and the implementation must see and use the names given by the implementation.
@scoder

This comment has been minimized.

Show comment
Hide comment
@scoder

scoder Sep 26, 2017

Contributor

Hmm, I cannot reproduce this. What I can see is that when using keyword arguments, it (now?) requires using the names declared in the .pxd file, which seems correct and consistent. I added a test that shows that the argument name used in the implementation is correctly defined: 1316eb2

Could you try to come up with a complete minimal example that fails for you?

Contributor

scoder commented Sep 26, 2017

Hmm, I cannot reproduce this. What I can see is that when using keyword arguments, it (now?) requires using the names declared in the .pxd file, which seems correct and consistent. I added a test that shows that the argument name used in the implementation is correctly defined: 1316eb2

Could you try to come up with a complete minimal example that fails for you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment