Skip to content

Correct code object invocation after semantic change in argcount for 3.8#2980

Merged
scoder merged 1 commit intocython:masterfrom
pablogsal:posonlyargcount_fix
Jun 1, 2019
Merged

Correct code object invocation after semantic change in argcount for 3.8#2980
scoder merged 1 commit intocython:masterfrom
pablogsal:posonlyargcount_fix

Conversation

@pablogsal
Copy link
Contributor

In python/cpython#13726 CPython has change the semantics of argcount to reflect all positional arguments (including positional-only). This is to make it easier to consumers to obtain the number of positional arguments if they do not care about positional-only arguments.

More information in:

https://mail.python.org/pipermail/python-dev/2019-June/157812.html
https://bugs.python.org/issue37122

CC: @scoder @serhiy-storchaka

@pablogsal
Copy link
Contributor Author

Seems like the test failures are unrelated to this change. Is the test suite ok?

@scoder
Copy link
Contributor

scoder commented Jun 1, 2019

I only see related failures. Where did you see an unrelated one? (master is ok)
Current CPython seems broken, so I can't currently test this locally either.

@scoder
Copy link
Contributor

scoder commented Jun 1, 2019

In any case, the change looks correct. I'll merge it and fix the corresponding tests.

@pablogsal
Copy link
Contributor Author

@scoder

I only see related failures. Where did you see an unrelated one? (master is ok)

My apologies, I was pretty confused why this PR is making Python2.7 fail, but I just realized Cython supports this now in Python2 compatible code, right?

@scoder scoder merged commit 8a910f4 into cython:master Jun 1, 2019
@pablogsal pablogsal deleted the posonlyargcount_fix branch June 1, 2019 19:19
@scoder
Copy link
Contributor

scoder commented Jun 1, 2019

Yes, we try to avoid making Cython features depend on CPython features wherever possible.

@pablogsal
Copy link
Contributor Author

Thank you very much for the review and for fixing the tests :)

@scoder
Copy link
Contributor

scoder commented Jun 1, 2019

The error that I saw in CPython is actually related to the change. We were previously adding up the pos-only count with the positional arguments, which made the consistency check in CPython's code object creation fail. So, all fine on all sides now.

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