Add tests for conversions in Python method calls #10840
Conversation
Adds tests for the following conversion scenarios: - Python list => .NET IList (CPython fails) - .NET array => Python list - Python tuple => .NET array - Python tuple => .NET IList (CPython fails) - Python range => .NET array - Python range => .NET IList (CPython fails) - Python dict => .NET IDictionary (CPython fails) - .NET IDictionary => Python dict (CPython unkonwn) Also commented out some tests that fail in both engines but may be considered to fix in CPython: - .NET IList => Python list - Python array => .NET IList - Python dict => DS Dictionary Other types considered but not added as tests: - classes (can't convert them) - dictionary view objects (uncommon) - bytes, bytearray, memoryview (uncommon) - set, frozenset (uncommon) - complex (uncommon)
|
Are these supported in IronPython:
|
|
@aparajit-pratap Yes. Where I wrote |
| # Python tuple => .NET array - Works in both | ||
| a = DummyCollection.MakeArray(t) | ||
| # Python tuple => .NET IList - Does not work in CPython | ||
| l = List.AddItemToEnd(4, t) |
There was a problem hiding this comment.
Are Python tuples mutable?
| r = range(0, 10, 2) | ||
| # Python range => .NET array - Works in both | ||
| a = DummyCollection.MakeArray(r) | ||
| # Python range => .NET IList - Does not work in CPython |
There was a problem hiding this comment.
Are Python ranges mutable? Do they result in lists or arrays? Please bear with the ignorant questions - I'm not a Python expert. Could there by differences in IronPython vs. CPython in the way that tuples, ranges, etc. behave in terms of mutability?
There was a problem hiding this comment.
Not problem at all. These are very good questions @aparajit-pratap .
Are Python ranges mutable?
What happens when a Python primitive gets passed to a .NET function call, is that the primitive gets converted to a .NET object. This means that in practice they are not mutable because what we get is not even in the Python world.
Do they result in lists or arrays?
This will depend on the declared type in the function call. Support for arrays was already built into Python.NET. For lists, a few minor changes were needed, but most of the logic can be reused.
Could there by differences in IronPython vs. CPython in the way that tuples, ranges, etc. behave in terms of mutability?
I don't think I have tested that. That's a very good point. I'll take a look at IronPython and see if I'm able to mutate a list for instance.
There was a problem hiding this comment.
I just checked and mutability can be achieved in IronPython for a List. You need to use a Zero Touch node to test it though, because List methods always make a copy.
| from FFITarget import DummyCollection | ||
|
|
||
| d = {'one': 1, 'two': 2, 'three': 3} | ||
| # Python dict => DS Dictionary - Does not work in either engine |
There was a problem hiding this comment.
I'm not sure if this is intended. It would be good to have but if it were to be supported, we would most likely need to make custom changes in pythonnet for interop with DS that would not be accepted by the main repo for back-porting and we would need to continue to maintain our own custom fork.
There was a problem hiding this comment.
We didn't do it for IronPython either.
There was a problem hiding this comment.
Thanks for the feedback. I thought of a way to do this without referencing DS Dictionary in Python.NET. What we would do is a two step conversion:
- Convert it to a .NET Dictionary, using primitives from Python.NET
- Convert the .NET Dictionary to a DS Dictionary, as part of a custom encoder in Dynamo
|
@mmisol thanks for answering my questions and for the detailed enumeration of all possible conversions and limitations. |
|
I'll go ahead and merge this. I'll link the task to the PR so that we can take a look whenever we see which of these scenarios we would like to address. |
Purpose
Adds tests for the following conversion scenarios:
Also commented out some tests that fail in both engines but may be
considered to fix in CPython:
Other types considered but not added as tests:
Other scenarios that might be interesting:
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@DynamoDS/dynamo