◐ Shell
reader mode source ↗
Skip to content

Ensure that param-array matching works correctly#1304

Merged
filmor merged 3 commits into
pythonnet:masterfrom
filmor:fix-params-index
Dec 10, 2020
Merged

Ensure that param-array matching works correctly#1304
filmor merged 3 commits into
pythonnet:masterfrom
filmor:fix-params-index

Conversation

@filmor

@filmor filmor commented Dec 3, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

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.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Add yourself to AUTHORS
  • Updated the CHANGELOG

@codecov-io

codecov-io commented Dec 3, 2020 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #1304 (13871f2) into master (f8c27a1) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1304   +/-   ##
=======================================
  Coverage   74.04%   74.04%           
=======================================
  Files           1        1           
  Lines         289      289           
=======================================
  Hits          214      214           
  Misses         75       75
Flag Coverage Δ
setup_windows 74.04% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8c27a1...13871f2. Read the comment docs.

@lostmsu

lostmsu commented Dec 3, 2020

Copy link
Copy Markdown
Member

Can you add a test with a mix of keyword and params array arguments?

@filmor

filmor commented Dec 4, 2020

Copy link
Copy Markdown
Member Author

I added some more tests, but three of them are failing. I commented them out for now and will try to fix them today.

lostmsu
lostmsu approved these changes Dec 9, 2020
View reviewed changes

@lostmsu lostmsu left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

Please, remove the commented out code first.

@filmor
Ensure that param-array matching works correctly …
001aeec 
We can match n Python parameters to exactly n+1 C# parameters of which
the last one is a param-array.

Fixes pythonnet#1302.
@filmor
Add tests for method matching in presence of param arrays
b9538b9
@filmor filmor mentioned this pull request Dec 10, 2020
Open
@filmor filmor merged commit 4133927 into pythonnet:master Dec 10, 2020
@filmor filmor deleted the fix-params-index branch December 10, 2020 17:47
@christabella christabella mentioned this pull request Dec 17, 2020
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lostmsu lostmsu lostmsu approved these changes

+1 more reviewer

@christabella christabella christabella left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@filmor @codecov-io @lostmsu @christabella