Ensure that param-array matching works correctly #1304
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1304 +/- ##
=======================================
Coverage 74.04% 74.04%
=======================================
Files 1 1
Lines 289 289
=======================================
Hits 214 214
Misses 75 75
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
|
Can you add a test with a mix of keyword and params array arguments? |
|
I added some more tests, but three of them are failing. I commented them out for now and will try to fix them today. |
We can match n Python parameters to exactly n+1 C# parameters of which the last one is a param-array. Fixes pythonnet#1302.
13871f2 to
b9538b9
Compare
| @@ -652,7 +652,7 @@ static Type TryComputeClrArgumentType(Type parameterType, IntPtr argument, bool | |||
| { | |||
| match = true; | |||
| } | |||
| else if (positionalArgumentCount < parameters.Length) | |||
| else if (positionalArgumentCount < parameters.Length && (!paramsArray || positionalArgumentCount == parameters.Length - 1)) | |||
There was a problem hiding this comment.
Maybe it would be useful to comment explicitly on what kind of cases would fall into this condition, or simplify the branching statements somehow (refactor the default arg logic in a separate method)? Just by reading this I'm guessing that this should take care of only the following 2 cases?
- when it's not a
paramsarray but there are some default values we need to fill in - when it's a
paramsarray and exactly one parameter is missing
Maybe there are more?
There was a problem hiding this comment.
Nevermind, actually I get it now! I also added an extra comment and renamed the variables for readability and consistency with the others parts of methodbinder in my PR: https://github.com/pythonnet/pythonnet/pull/1324/files#diff-237dc718ad5424f5476d2ebbbc4e5164c95901bd2a4c2daaf8ca6fc9f9d3a5fbR703-R704
What does this implement/fix? Explain your changes.
The previous matching code wasn't specific enough for param arrays, the count of parameters has to be exactly right.
Does this close any currently open issues?
#1302
Any other comments?
Checklist
Check all those that are applicable and complete.
AUTHORSCHANGELOG