Marshall out iterables as lists in CPython #10894

mmisol · 2020-07-15T21:18:55Z

Purpose

Adds support for marshalling out iterables that are not lists or
dictionaries using the CPython engine. In order to achieve this, the
check PyIter.IsIterable was used, along with some extra considerations,
mentioned in the source with comments.

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

@mjkkirschner

FYIs

@DynamoDS/dynamo

Adds support for marshalling out iterables that are not lists or dictionaries using the CPython engine. In order to achieve this, the check PyIter.IsIterable was used, along with some extra considerations, mentioned in the source with comments.

mjkkirschner · 2020-07-15T21:25:26Z

test/Libraries/DynamoPythonTests/PythonEvalTests.cs

@@ -195,5 +195,34 @@ class myobj:
                Assert.AreEqual("Output could not be converted to a .NET value", exc.Message);
            }
        }
+
+        [Test]
+        public void NonListIterablesCanBeOutput()


does this test hit the condition on line 204?

Not this test. One which failed without that part is this one https://github.com/DynamoDS/Dynamo/blob/master/test/Libraries/DynamoPythonTests/PythonEvalTestsWithLibraries.cs#L189

so it's currently failing?

No, sorry if I didn't say it right. That test does work and hits that line.

What I meant is that at some point during my development of this I noticed the test failed, which led me to add the block that spans from line 201 to 205.

mjkkirschner

LGTM

mmisol · 2020-07-15T22:03:48Z

Tests passed here: https://master-15.jenkins.autodesk.com/view/DYN/job/DYN-DevCI_Self_Service/707/. Merging

mmisol added 3 commits July 15, 2020 10:27

Attempt to simplify (does not work)

b1b4539

Merge branch 'master' into cpython_output_iterables

4c85af1

mmisol requested a review from mjkkirschner July 15, 2020 21:18

mjkkirschner reviewed Jul 15, 2020

View reviewed changes

mjkkirschner approved these changes Jul 15, 2020

View reviewed changes

mmisol merged commit 0da94b4 into DynamoDS:master Jul 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Marshall out iterables as lists in CPython #10894

Marshall out iterables as lists in CPython #10894

Uh oh!

mmisol commented Jul 15, 2020 •

edited

Loading

Uh oh!

mjkkirschner Jul 15, 2020

Uh oh!

mmisol Jul 15, 2020

Uh oh!

mjkkirschner Jul 15, 2020

Uh oh!

mmisol Jul 15, 2020 •

edited

Loading

Uh oh!

mjkkirschner left a comment

Uh oh!

mmisol commented Jul 15, 2020

Uh oh!

Uh oh!

Marshall out iterables as lists in CPython #10894

Marshall out iterables as lists in CPython #10894

Uh oh!

Conversation

mmisol commented Jul 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Declarations

Reviewers

FYIs

Uh oh!

mjkkirschner Jul 15, 2020

Choose a reason for hiding this comment

Uh oh!

mmisol Jul 15, 2020

Choose a reason for hiding this comment

Uh oh!

mjkkirschner Jul 15, 2020

Choose a reason for hiding this comment

Uh oh!

mmisol Jul 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjkkirschner left a comment

Choose a reason for hiding this comment

Uh oh!

mmisol commented Jul 15, 2020

Uh oh!

Uh oh!

mmisol commented Jul 15, 2020 •

edited

Loading

mmisol Jul 15, 2020 •

edited

Loading