gh-156500: Reject non-positional parameters in wrap_dll_function() - #156501
gh-156500: Reject non-positional parameters in wrap_dll_function()#156501fedonman wants to merge 2 commits into
Conversation
Documentation build overview
|
ZeroIntensity
left a comment
There was a problem hiding this comment.
Let's raise an exception for these rather than skipping them. Remember, errors should never pass silently.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
argtypes was built from every annotated parameter, so an annotated keyword-only, *args or **kwargs parameter contributed an extra positional entry. Such a parameter has no positional counterpart to describe, so it now raises ValueError at decoration time.
77f100d to
453280a
Compare
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @ZeroIntensity: please review the changes made to this pull request. |
| :exc:`ValueError` is raised. If the name of the function does not exist | ||
| in *dll*, an :exc:`AttributeError` is raised. | ||
| :exc:`ValueError` is raised. A :exc:`ValueError` is also raised if it has a | ||
| keyword-only, ``*args`` or ``**kwargs`` parameter, since |
There was a problem hiding this comment.
The Oxford comma significantly improves clarity here:
| keyword-only, ``*args`` or ``**kwargs`` parameter, since | |
| keyword-only, ``*args``, or ``**kwargs`` parameter, since |
| def test_wrap_dll_function_non_positional(self): | ||
| # argtypes describes positional arguments only, so a parameter that | ||
| # cannot be passed positionally is rejected. |
There was a problem hiding this comment.
Can you also test that positional-only arguments (def PyObject_GetAttr(op: ctypes.py_object, attr: ctypes.py_object, /)) do not raise an exception?
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @ZeroIntensity: please review the changes made to this pull request. |
argtypeswas built from every annotated parameter, so an annotated keyword-only,*argsor**kwargsparameter contributed an extra positional entry. Such a parameter has no positional counterpart to describe, so it now raisesValueErrorat decoration time.No NEWS entry:
wrap_dll_functionis new in 3.16 and has not been released, so this is folded into the original entry and needs theskip newslabel.wrap_dll_functionbuildsargtypesfrom non-positional parameters #156500