Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decode Python Dictionary in function calls #10868

Merged
merged 5 commits into from
Jul 10, 2020

Conversation

mmisol
Copy link
Collaborator

@mmisol mmisol commented Jul 8, 2020

Purpose

Allows to decode Python dict to .NET IDictionary by using a Python.NET
decoder. Both generic and non-generic dictionaries are supported.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.

Reviewers

@DynamoDS/dynamo

mmisol added 5 commits July 2, 2020 17:12
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)
@mmisol mmisol requested a review from a team July 8, 2020 21:15
var decoders = shared.Cast<IPyObjectDecoder>().Concat(new IPyObjectDecoder[]
{
new DictionaryDecoder()
}).ToArray();
Copy link
Contributor

Choose a reason for hiding this comment

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

@mmisol so it looks like we have both an encoder and a decoder for List but not for Dictionary, correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that's technically correct. In practice, the encoder we have for List only has the purpose of preventing the default encoding Python.NET does for lists. By doing that we can get a consistent behaviour between List and Dictionary conversions.

@@ -6,7 +6,7 @@

namespace DSCPython.Encoders
{
internal class ListEncoder : IPyObjectEncoder, IPyObjectDecoder
internal class ListEncoderDecoder : IPyObjectEncoder, IPyObjectDecoder
Copy link
Contributor

Choose a reason for hiding this comment

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

I see obsolete messages on IPyObjectEncoder and Decoder saying they are unstable and can be changed in the next minor release - something to keep note of unless there is an alternative interface already existing that we can use and replace right away.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The custom encoder/decoder API appeared with 2.5, which is the latest minor release of Python.NET. My guess is they might make changes to it in version 3.0 so you bring a good point.

Before this API there was no way to do something like this AFAIK. We chose to use the API where possible, even if it is unstable, because otherwise we would have to modify Python.NET even more. Also, from these encoders/decoders you could target custom types, like DS Dictionary, which is another plus.


OUT = d
# Python dict => .NET IDictionary<> - Does not work in IronPython
typedDictionary = DummyCollection.AcceptDictionary(d)
Copy link
Contributor

@aparajit-pratap aparajit-pratap Jul 9, 2020

Choose a reason for hiding this comment

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

I understand that passing d to AcceptDictionary or AcceptIDictionary is the decoding part to convert it into the .NET dictionary but what about returning the .net dictionary from AcceptDictionary? Doesn't that need an encoding from the returned .NET dictionary to the Python dictionary?

Copy link
Contributor

Choose a reason for hiding this comment

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

Are the returned untypedDictinoary and typedDictionary not Python dictionaries?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It does trigger an encode operation but in this case we are not doing anything special. Python.NET will wrap .NET objects that it doesn't know how to encode with a generic wrapper. I haven't taken much of a look to their implementation, but they even support iterating like this for example https://github.com/DynamoDS/Dynamo/blob/master/test/Libraries/DynamoPythonTests/PythonEvalTestsWithLibraries.cs#L77

@mmisol mmisol merged commit b58f0d9 into DynamoDS:master Jul 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants