Convert BigInteger to Int64 when possible from Python

[aparajit-pratap]

Purpose

https://autodesk.slack.com/archives/CED3BF6C9/p1571942488031200

JIRA: https://jira.autodesk.com/browse/DYN-2262

Declarations

Check these if you believe they are true

• The code base 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

FYIs

@Dewb

[Dewb]

Excellent!

Any reason this has to be done inside the generic marshaller execution rather than by defining a custom marshaler for BigInteger?

[Dewb]

Looks like we have custom DataMarshalers for Python node inputs, but not outputs. This new code could definitely go here: https://github.com/DynamoDS/Dynamo/blob/master/src/Libraries/DSIronPython/IronPythonEvaluator.cs

The DataMarshaler class isn't used anywhere aside from the Python node so I would also support moving it out of DynamoUtilities and into the Python project, to make its purpose more clear. (Doesn't have to be part of this PR.)

[aparajit-pratap]

Looks like we have custom DataMarshalers for Python node inputs, but not outputs. This new code could definitely go here: https://github.com/DynamoDS/Dynamo/blob/master/src/Libraries/DSIronPython/IronPythonEvaluator.cs

The DataMarshaler class is used for both input and output marshaling as seen here:

Dynamo/src/Libraries/DSIronPython/IronPythonEvaluator.cs

Line 114 in 8fb3b68

scope.SetVariable((string)bindingNames[i], InputMarshaler.Marshal(bindingValues[i]));

and here:

Dynamo/src/Libraries/DSIronPython/IronPythonEvaluator.cs

Line 134 in 8fb3b68

return OutputMarshaler.Marshal(result);

I agree that the marshaling for BigInteger can be specific to the output marshaling. Will make the necessary changes.

[mjkkirschner]

@aparajit-pratap would you mind filing this internally?

[aparajit-pratap]

Added unit test, addressed review comments about registering the BigInteger marshaling logic only for node output (output marshalers are added only in the DataMarshaler constructor, while input marshalers are added when first initializing the IronPythonEvaluator property).

I haven't moved DataMarshaler to the Python project just yet as @mjkkirschner had some other thoughts on this. Leaving it open for discussion. Aside from this, the PR is ready to be merged.

[QilongTang]

LGTM with one comment

[Dewb]

FYI the IronPython issue that prompted this is supposedly fixed in IronPython 2.7.9.

[mjkkirschner]

hmm - so this test should have passed already - since we are using ironPython 2.7.9.

@aparajit-pratap do we now include system.numerics in our shipped binaries or this is loaded from the GAC?

It will be useful to know as we should communicate this in the release notes of 2.5

[aparajit-pratap]

I reverted my change leaving the test to test locally. It didn't pass.

[mjkkirschner]

@aparajit-pratap your test also includes a big integer check (last type in your list) - perhaps that gets marshalled to long now in all cases?

[mjkkirschner]

here's Mike's issue:
IronLanguages/ironpython2#681

[aparajit-pratap]

@aparajit-pratap your test also includes a big integer check (last type in your list) - perhaps that gets marshalled to long now in all cases?

This is the output I'm seeing before my change:

[aparajit-pratap]

@aparajit-pratap do we now include system.numerics in our shipped binaries or this is loaded from the GAC?

It is loaded from GAC and referenced from C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.7\