Fix #1523 #1524
Fix #1523 #1524
Conversation
|
This fails some tests. |
|
It looks like master is also failing the tests? I may be wrong since I only looked on my phone. I have not tried removing needsResolution, I can try that. |
|
@slide master fails pretty rarely, and only in 1 configuration, probably due to heap corruption. The failure here is directly related to the change. |
|
Ok, I'll definitely take a look. I need to stop using my phone for looking at github |
|
Ok, I can see the failures much easier on my computer. It looks like a different overload is being chosen for the <= operator. I'll track down the issue and see why it is happening. |
This sorts matching methods so that non PyObject overloads are looked at first. The other option for this would be to change the test such that it determines the type before assuming that it is a tuple (See public static bool operator <=(OperableObject a, PyObject b) int TestOperator.cs)
|
I don't know if the solution I proposed is a good one for all circumstances. Another way of fixing the tests would be to change the overload that takes a PyObject to not assume that it is a tuple. |
| // this when there is more than one arg matched method | ||
| if (argMatchedMethods.Count > 1) | ||
| { | ||
| argMatchedMethods = argMatchedMethods.OrderBy(x => x.HasPyObjectParameter).ToList(); |
There was a problem hiding this comment.
What is the scenario? Why is this change needed?
|
In the case of the test, there are two overloads that could match, one that has (OperableObject, int) and one that has (OperableObject, PyObject) as parameters. The one with PyObject is "matched" first and all the arguments can be coerced to the required parameters, so this method is chosen as the matching method to execute. That overload assumes that the PyObject argument is a tuple rather than a simple integer. This change sorts the matching methods so that ones with PyObject parameters would be checked last. As I said in my last comment, I don't know if this is a good solution, or if I should just change the test. It seems like users would want more specific methods matched first before PyObject parameter ones, but again, I am not sure. |
|
Closed in favor of #1531 |
What does this implement/fix? Explain your changes.
Adds a preferred type parameter to GetTypeByAlias and then the preferred type gets passed in in methodbinder's TryComputeClrArgumentType. I am not sure if this is the best fix for the issue, I know that the test fails before the fix and passes after.
Does this close any currently open issues?
#1523
Checklist
Check all those that are applicable and complete.
AUTHORSCHANGELOG