QuantConnect Update #1385

C-SELLERS · 2021-02-15T23:49:46Z

What does this implement/fix? Explain your changes.

This PR seeks to bring QuantConnect/PythonNet 's changes to the root repo.

Since QuantConnect's fork was so distant from the root repo it required that I reapply all the needed changes in order to keep desired functionality. To denote this in the commits I referenced the PR I was trying to replicate. Please be aware that Github will link these to the root #n PR but they are all referencing QuantConnect/PythonNet PRs

Any other comments?

QuantConnect Changes Reflected (Reference QuantConnect/PythonNet for original PR):

Support decimal, timespan, datetime, implicit, and nullable conversions (Support for decimal and fixing build issues in pythonnet QuantConnect/pythonnet#1, Implements System.Decimal and System.DateTime support QuantConnect/pythonnet#8, Method overload with decimal parameter accepts non-decimal numbers QuantConnect/pythonnet#10, Fixing memory leaks in the conversion of some types QuantConnect/pythonnet#14, Do not include timezone if DateTimeKind is Unspecified QuantConnect/pythonnet#15, Decimal Parsing Allows Numeric String in Exponential Notation and Version Bump QuantConnect/pythonnet#25)
Use Float in Python for Decimal conversion (Performance improvement C# decimal to Python conversion QuantConnect/pythonnet#19)
Some logging on boot up (Version bump 1.0.5.23 - Init logs QuantConnect/pythonnet#34)
Support List conversion (PyList to C# List round trip conversion QuantConnect/pythonnet#37)
AssemblyManager Cache (Performance improvement Initialization hang fix QuantConnect/pythonnet#38)
Generic Util race condition fix (Initialization hang fix QuantConnect/pythonnet#38)
Support KeyValuePairEnumerable Objects (Implements KeyValuePairEnumerableObject QuantConnect/pythonnet#42)

Patches to address failures in Lean:

MethodBinder changes to allow implicit conversion. This one was particularly tricky and seemed impossible to patch together with the MethodBinder.cs in the root. Unfortunately it involved tearing apart a lot of it and rebuilding only what was needed. It may be possible to refactor this to look more like the root and still pass our tests. ( 58d5df0 , 44e089a , 108eacf , 6379568 , 9f2796a )
Revert PythonNet PR Wrap returned objects in interface if method return type is interface #1240 ; this PR broke fetching interface derived classes data members that weren't defined in the interface. ( ed6ab18 )
Converting PyObject to Int fix; this reflects the way PythonNet used to convert values to Int32, 64, unsigned etc, with PyNumber_Long but still keeps the use of Runtime.PyLong_AsSignedSize_t to finish the conversion. The bug that discovered this was trying to convert a Python float to int, this used to work but was failing until this refactor. Reference the added test in bd94e49. ( d87584b )

Tests added:

TimeDelta and DateTime Conversion Tests ( 508db2e )
List Conversion tests ( 5839d21 )
Dictionary Tests ( 101624e )
MethodBinder Tests ( bf1755d )
OperatorInequality Test ( b0aca5c )
PrimitiveIntConversion Test ( bd94e49 )
Interface Classes Test ( cd06d10 )

Some tests had to be adjusted for behavior changes caused by our commits. I have adjusted enough to have all PythonNet embedded tests passing, but there are still some issues with PyTests that need to be addressed/resolved.

I'm aware this a huge collection of changes, so I don't expect this to be easy but would hope maybe there is a way to bring our branches together so we can help improve PythonNet in the future.

Also note this branch is also a pending PR to QuantConnect/PythonNet . Reference this PR at QuantConnect#47

Checklist

Check all those that are applicable and complete.

Make sure to include one or more tests for your change
If an enhancement PR, please create docs and at best an example
Add yourself to AUTHORS
Updated the CHANGELOG

…erface" This reverts commit 50d947f, reversing changes made to d44f1da.

dnfadmin · 2021-02-15T23:50:02Z

All CLA requirements met.

filmor · 2021-02-16T09:50:22Z

Thank you for taking this up again.

Support decimal, timespan, datetime, implicit, and nullable conversions (QuantConnect#1, QuantConnect#8, QuantConnect#10, QuantConnect#14, QuantConnect#15, QuantConnect#25)

Use Float in Python for Decimal conversion (Performance improvement QuantConnect#19)

I'm still very much against doing these conversions without a way to shut them off (while I agree that they might be good defaults). We nowadays have a system to implement additional conversions and configure them at runtime, codecs for the types you listed would be appreciated: https://github.com/pythonnet/pythonnet/wiki/Codecs:-customizing-object-marshalling-between-.NET-and-Python

Some logging on boot up (QuantConnect#34)
Straight Console.Writes will not get accepted. Trace logging might be, as long as it doesn't pull in additional dependencies (e.g. https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.trace?view=net-5.0)

Support List conversion (QuantConnect#37)

This might also be implementable as a codec. I'll have a closer look

AssemblyManager Cache (Performance improvement QuantConnect#38)

This looks nice, needs a separate PR.

Generic Util race condition fix (QuantConnect#38)
Is this still applicable?

Support KeyValuePairEnumerable Objects (QuantConnect#42)

Also looks like a codec candidate.

MethodBinder changes to allow implicit conversion. This one was particularly tricky and seemed impossible to patch together with the MethodBinder.cs in the root. Unfortunately it involved tearing apart a lot of it and rebuilding only what was needed. It may be possible to refactor this to look more like the root and still pass our tests. ( 58d5df0 , 44e089a , 108eacf , 6379568 , 9f2796a )

This would definitely need a separate PR. You could start with a PR that adds the tests.

Revert PythonNet PR Wrap returned objects in interface if method return type is interface #1240 ; this PR broke fetching interface derived classes data members that weren't defined in the interface. ( ed6ab18 )

Could you give a concrete example of what is failing now? Can it be mitigated using the __implementation__ member?

Converting PyObject to Int fix; this reflects the way PythonNet used to convert values to Int32, 64, unsigned etc, with PyNumber_Long but still keeps the use of Runtime.PyLong_AsSignedSize_t to finish the conversion. The bug that discovered this was trying to convert a Python float to int, this used to work but was failing until this refactor. Reference the added test in bd94e49. ( d87584b )

This looks good, can you create a PR for this one?

koubaa · 2021-02-16T20:28:21Z

@filmor @C-SELLERS I sent an email to the pythonnet group about issues coming from #1240. @lostmsu suggested some things (here are 4 and 5 of his suggestions):

"
4. IronPython apparently allows explicit interface implementations to be called (instead of returning interface-wrapped instances), if there’s no conflict with regular methods. Perhaps we should consider this as an option.
5. Alternatively, we can keep wrapped instances, but allow to resolve public members too if the method is not found in the interface. I like this option better than 4, because actual type of an element in ISomething[] won’t change behavior of invoking interface member. It might still affect hasattr, but this is somewhat expected."

I think (4) is something like a revert.

lostmsu · 2021-02-17T03:39:02Z

Converting PyObject to Int fix; this reflects the way PythonNet used to convert values to Int32, 64, unsigned etc, with PyNumber_Long but still keeps the use of Runtime.PyLong_AsSignedSize_t to finish the conversion. The bug that discovered this was trying to convert a Python float to int, this used to work but was failing until this refactor. Reference the added test in bd94e49. ( d87584b )

This looks good, can you create a PR for this one?

The old behavior (implicit conversions between numerical types like in the new test) should not be restored. I suggest you require explicit conversion via int(decimal_value) on Python side or a similar method. Implicit conversion is dangerous (especially in finance).

lostmsu · 2021-02-17T05:30:11Z

I'm still very much against doing these conversions without a way to shut them off (while I agree that they might be good defaults). We nowadays have a system to implement additional conversions and configure them at runtime, codecs for the types you listed would be appreciated: https://github.com/pythonnet/pythonnet/wiki/Codecs:-customizing-object-marshalling-between-.NET-and-Python

We also have a repository for community-contributed codecs: https://github.com/pythonnet/codecs

C-SELLERS · 2021-02-19T21:36:04Z

Thank you guys for your feedback!

I plan to look into these when permitted and hope to start PR'ing the changes individually that can work! I definitely need to take some time and look at the codec setup you guys have, I wasn't quite sure how it worked and I didn't really understand the wiki either. I'll search for some examples and test it out and see if our conversions can be done that way.

C-SELLERS and others added 30 commits February 15, 2021 13:13

Reflect PR #1 Support for Decimal

062a0bc

Reflect PR#8 MISSING CONVERTER.CS L516-528 Changes

5ed5a96

Reflect PR #14

9b5445e

Reflect PR #15

709643a

Reflect PR #19

a481700

Reflect PR #25

de0328c

Reflect PR #34

481f4d0

Reflect PR #35

863530d

Implement List Conversion, Reflect PR #37 Tests

5839d21

Reflect PR #38 Partial: Assembly Manager Improvements

c362816

Reflect PR #38

cde9b51

Reflect PR #42 KeyValuePairEnumerableObject

101624e

Reflect PR #10 Runtime DecimalType

56a4bcf

Add TimeDelta and DateTime tests

508db2e

Fix DecimalConversion test for float conversion

ebbafad

Converter mod tweaks

53375ce

Adjust a few broken PyTests

c8fdbcb

Use _pydecimal to not interfere with Lean/decimal.py

af30873

Add MethodBinder tests

bf1755d

MethodBinder implicit resolution

58d5df0

Fix bad cherry pick

0c94228

Refactoring precedence resolution

44e089a

Deal with operator binding

108eacf

Fix TestNoOverloadException unit test

6379568

Fix for DomainReload tests

9f2796a

Add InEquality Operator Test

b0aca5c

Dont PyObjects precedence in Operator methods

0f5f0ba

Revert "Merge pull request #1240 from danabr/auto-cast-ret-val-to-int…

ed6ab18

…erface" This reverts commit 50d947f, reversing changes made to d44f1da.

Fix Primitive Conversion to Int

d87584b

Post rebase fix

f59335f

C-SELLERS added 3 commits February 15, 2021 16:00

Add PrimitiveIntConversion test

bd94e49

Add test for interface derived classes

cd06d10

Add to Authors.md

aeb20c0

C-SELLERS marked this pull request as ready for review February 16, 2021 00:23

Load in current directory into Python Path

ae34f30

C-SELLERS closed this Feb 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QuantConnect Update #1385

QuantConnect Update #1385

C-SELLERS commented Feb 15, 2021 •

edited by filmor

Loading

dnfadmin commented Feb 15, 2021 •

edited

Loading

filmor commented Feb 16, 2021 •

edited

Loading

koubaa commented Feb 16, 2021

lostmsu commented Feb 17, 2021 •

edited

Loading

lostmsu commented Feb 17, 2021

C-SELLERS commented Feb 19, 2021

QuantConnect Update #1385

QuantConnect Update #1385

Conversation

C-SELLERS commented Feb 15, 2021 • edited by filmor Loading

What does this implement/fix? Explain your changes.

Any other comments?

Checklist

dnfadmin commented Feb 15, 2021 • edited Loading

filmor commented Feb 16, 2021 • edited Loading

koubaa commented Feb 16, 2021

lostmsu commented Feb 17, 2021 • edited Loading

lostmsu commented Feb 17, 2021

C-SELLERS commented Feb 19, 2021

C-SELLERS commented Feb 15, 2021 •

edited by filmor

Loading

dnfadmin commented Feb 15, 2021 •

edited

Loading

filmor commented Feb 16, 2021 •

edited

Loading

lostmsu commented Feb 17, 2021 •

edited

Loading