NotificationsYou must be signed in to change notification settings
Fork750
Star5.1k

Codecs: customize set of Py<->C# conversions via PyObjectConversions#1022

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

lostmsu merged 17 commits intopythonnet:masterfromlosttech:PR/Codecs

Feb 26, 2020

Merged

Codecs: customize set of Py<->C# conversions via PyObjectConversions#1022

lostmsu merged 17 commits intopythonnet:masterfromlosttech:PR/Codecs

Feb 26, 2020

Conversation

Copy link

Member

lostmsu commentedDec 19, 2019•
edited
Loading

What does this implement/fix? Explain your changes.

This enables registering new automatic Py->C# and C#->Py conversions (codecs), that will run when Python calls .NET code and vice versa.

Does this close any currently open issues?

This is a new feature

Any other comments?

...

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: seeTupleCodec
Add yourself toAUTHORS
Updated theCHANGELOG

lostmsu force-pushed thePR/Codecs branch 2 times, most recently frome840a4b to3dff8b5Compare

December 20, 2019 02:20

lostmsu mentioned this pull request

Dec 20, 2019

Improve method binding#974

Closed

4 tasks

koubaa reviewed

Dec 21, 2019

View reviewed changes

src/embed_tests/Python.EmbeddingTest.15.csproj

		@@ -23,7 +23,7 @@
		<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == 'Undefined'">..\..\</SolutionDir>
		<PythonBuildDir Condition="'$(TargetFramework)'=='net40' AND '$(PythonBuildDir)' == ''">$(SolutionDir)\bin\</PythonBuildDir>
		<PublishDir Condition="'$(TargetFramework)'!='net40'">$(OutputPath)\$(TargetFramework)_publish</PublishDir>
		<LangVersion>6</LangVersion>
		<LangVersion>7.3</LangVersion>

Copy link

Contributor

Choose a reason for hiding this comment

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

Is this intentional?@lostmsu

Copy link

MemberAuthor

lostmsuDec 22, 2019•
edited
Loading

Choose a reason for hiding this comment

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

Yes, I think I am using a few features. Is there a C# compiler, that does not support 7.3 yet?
It is 7.3 in Runtime for a few months now.

Copy link

Contributor

Choose a reason for hiding this comment

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

@lostmsu just wanted to make sure it wasn't added by mistake.

koubaa reviewed

Jan 15, 2020

View reviewed changes

src/runtime/runtime.cs

		/// <summary>
		/// Increase Python's ref counter for the given object, and get the object back.
		/// </summary>
		internal static IntPtr SelfIncRef(IntPtr op)

Copy link

Contributor

Choose a reason for hiding this comment

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

This is neat and might simplify other code in the project

Copy link

Contributor

I like the conciseness of the terminology (encode/decode) but for me it isn't entirely obvious which direction (py <-> C#) is encoding and decoding. It is ok with me as long as we document it well enough.

Which other encoders should exist inside of Python.Runtime aside from the Tuple one you've added? Would you be OK with the List and Action ones I am interested in eventually being distributed by PythonNet?

I am writing the below snippet to document how it'd be available for external codecs to see how we like the syntax. To me it is not too bad:

import clrclr.AddReference("MyCodecs")import MyCodecsfor encoder in MyCodecs.Encoders:    Python.Runtime.PyObjectConversions.RegisterEncoder(encoder)for decoder in MyCodecs.Decoders:    Python.Runtime.PyObjectConversions.RegisterDecoder(decoder)

koubaa approved these changes

Jan 15, 2020

View reviewed changes

koubaa approved these changes

Jan 15, 2020

View reviewed changes

src/runtime/converter.cs

		@@ -135,8 +135,16 @@ internal static IntPtr ToPython(object value, Type type)
		return result;
		}

		if (value is IList && !(value is INotifyPropertyChanged) && value.GetType().IsGenericType)
		{
		if (Type.GetTypeCode(type) == TypeCode.Object && value.GetType() != typeof(object)) {

Copy link

Contributor

koubaaJan 15, 2020

Choose a reason for hiding this comment

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

glad this comes before other builtin conversions but ultimately the built-in conversions should probably use this codec system internally.

koubaa reviewed

Jan 15, 2020

View reviewed changes

src/runtime/converterextensions.csShow resolvedHide resolved

lostmsu requested a review fromfilmor

January 16, 2020 19:05

Copy link

MemberAuthor

lostmsu commentedJan 16, 2020

Pushed a fix to failing test

filmor approved these changes

Jan 22, 2020

View reviewed changes

src/runtime/Codecs/TupleCodecs.cs Outdated


		public bool CanEncode(Type type)
		=> type.Namespace == typeof(TTuple).Namespace && type.Name.StartsWith(typeof(TTuple).Name + '`')
		\|\| type == typeof(object) \|\| type == typeof(TTuple);

Copy link

Member

filmorJan 17, 2020

Choose a reason for hiding this comment

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

Maybe write this one out as a normal function block, it's a bit tedious to read.

Copy link

Member

filmorJan 17, 2020

Choose a reason for hiding this comment

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

Also, can't you just usetype.IsGenericType && type.GetGenericTypeDefinition() == typeof(TTuple) or something like that?

lostmsu force-pushed thePR/Codecs branch fromdf98205 toa1f4affCompare

January 30, 2020 01:56

lostmsu added3 commits

January 29, 2020 20:12

enable expanding set of marshaling conversions via PyObjectConversions

28143d5

added sample TupleCodec (only supporting ValueTuple)

fixed ConversionsObject test failing due to sequence to array convers…

6eca169

…ion taking priority

attempt to fix CI build issue with ValueTuple under Mono

97e33c7

lostmsu force-pushed thePR/Codecs branch froma1f4aff to97e33c7Compare

January 30, 2020 04:12

added RefereneAssemblies package reference to fix CI build

449338f

Copy link

MemberAuthor

lostmsu commentedJan 30, 2020

Mark methods as obsolete until they are stable.

lostmsu added2 commits

January 31, 2020 20:35

marked the new codecs API as unstable

39b2347

attempt to fix PyScopeTest.TestThread() reading stale value from res …

daa2901

…in Python 2.7 x64

lostmsu force-pushed thePR/Codecs branch from4b94a46 todaa2901Compare

February 1, 2020 06:43

Copy link

codecov-io commentedFeb 1, 2020•
edited
Loading

Codecov Report

Merging#1022 intomaster willnot change coverage.
The diff coverage isn/a.

@@           Coverage Diff           @@##           master    #1022   +/-   ##=======================================  Coverage   86.75%   86.75%           =======================================  Files           1        1             Lines         302      302           =======================================  Hits          262      262             Misses         40       40

Flag	Coverage Δ
#setup_linux	`65.56% <ø> (ø)`	⬆️
#setup_windows	`71.52% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last update5619fb9...41de69d. Read thecomment docs.

Copy link

MemberAuthor

lostmsu commentedFeb 13, 2020

@filmor I think this can be merged

lostmsuand others added6 commits

February 12, 2020 18:28

Merge branch 'master' into PR/Codecs

e8e3b4b

Merge branch 'master' into PR/Codecs

8818610

Merge branch 'master' into PR/Codecs

82f6b99

fixed bad IncRef after PyTuple_New

ec98209

corrected reference counting in Codecs

50a3822

don't dispose encoded object in case codec keeps a cache of them

2e19f2c

filmorand others added4 commits

February 23, 2020 18:05

Merge branch 'master' into PR/Codecs

44bfec2

do not dispose object, that might have been just decoded succesfully,…

399ae54

… as the decoded object might store a reference to the original PyObject

remove incref for tuple fields, as Converter.ToPython is supposed to …

e2d3333

…return a new reference

Merge branch 'master' into PR/Codecs

5619fb9

lostmsu force-pushed thePR/Codecs branch from04b539e to5619fb9Compare

February 25, 2020 22:28

Merge branch 'master' into PR/Codecs

41de69d

Copy link

MemberAuthor

lostmsu commentedFeb 26, 2020

Either I got lucky in this run, or#1050 actually was the real cause for test failures in this branch, and so was fixed by#1062.

lostmsu merged commit4d1442d intopythonnet:master

Feb 26, 2020

amos402 reviewed

Feb 27, 2020

View reviewed changes

src/embed_tests/TestPyScope.csShow resolvedHide resolved

This was referencedFeb 27, 2020

Enable user classes to override how Python.NET processes parameters of their functions#835

Closed

TestThread does not synchronize properly#1067

Closed

Copy link

Member

amos402 commentedFeb 28, 2020

The code style of this PR is totally different by the current code, is it really good for this?

lostmsu deleted the PR/Codecs branch

March 7, 2020 21:19

lostmsu mentioned this pull request

May 5, 2020

A way to override Python.NET .NET <-> Python wrapping for custom classes#823

Closed

Copy link

igiona commentedSep 16, 2021

The issue#823 has been marked as solved by this PR.
It's not clear to me which approach has been taken here (from the ones described in the mentioned bug)?
@lostmsu is the PR going to make Python.NET fully support .NET objects providing theIDynamicMetaObjectProvider interface? I'm moving away from IronPython (which supportsIDynamicMetaObjectProvider objects) and running into many issues...

From what I can see, this new feature didn't make it to the latest release, is there a plan for for a 2.5.3 or there will be only the v3.0.0?
Thanks

Copy link

MemberAuthor

lostmsu commentedSep 16, 2021•
edited
Loading

@igiona There's no support forIDynamicMetaObjectProvider on custom .NET objects. Instead for custom types to be presented to Python (or vice versa) as something they are not, you need to implement encoders and/or decoders:customizing object marshalling between .NET and Python

Copy link

igiona commentedSep 17, 2021

Thank you@lostmsu for the quick answer.
This implies that the PythonNet assemblies needs to imported in the .NET application, which is not always possible/desirable (not technically, but rather "politically"/by-design.

What's the rational behind the decision to not supportIDynamicMetaObjectProvider ?
Is there some architectural constraint that prevent this, or it has a chance to get supported by Python.NET in the future?

Copy link

MemberAuthor

lostmsu commentedSep 17, 2021•
edited
Loading

The rational is that pythonnet is an open source project, and nobody stepped up to implement this feature.

I am not sure what you mean by "PythonNet assemblies needs to imported in the .NET application, which is not always possible/desirable". You can develop and register codecs from Python (though development in Python might be tricky). You can also develop codecs in C#, compile them to a DLL, and register them from it. e.g.

clr.AddReference('MyCodecs.dll')fromMyCodecsimportMyMegaCodecForTypeXcodec=MyMegaCodecForTypeX()PyObjectConversions.RegisterEncoder(codec)PyObjectConversions.RegisterDecoder(codec)...workwithTypeX ...

Copy link

igiona commentedSep 19, 2021•
edited
Loading

The rational is that pythonnet is an open source project, and nobody stepped up to implement this feature.

This makes sense 👍

I did not checked yet the implementation above into details, but I assumed that the codecs have to implement the interfacesIPyObjectDecoder and/orIPyObjectEncoder in order to be able to be registered viaRegisterEncoder/RegisterDecoder.
Which means that my C#-application (which provides these codec) needs to import the PaythonNET assemblies as dependency in order to compile ("which is not always possible/desirable"). Unless your implementation looks for the interface method names by reflection and wraps/maps them to the PythonNET interfaces internally!?

Copy link

MemberAuthor

lostmsu commentedSep 19, 2021

Again, the app does not have to import pythonnet assemblies. Codecs can be put into a separate library.

Labels

None yet

6 participants

Movatterモバイル変換

Codecs: customize set of Py<->C# conversions via PyObjectConversions#1022

Codecs: customize set of Py<->C# conversions via PyObjectConversions#1022

Uh oh!

Conversation

lostmsu commentedDec 19, 2019• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

What does this implement/fix? Explain your changes.

Does this close any currently open issues?

Any other comments?

Checklist

Uh oh!

koubaaDec 21, 2019

Choose a reason for hiding this comment

Uh oh!

lostmsuDec 22, 2019• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

koubaaJan 15, 2020

Choose a reason for hiding this comment

Uh oh!

koubaaJan 15, 2020

Choose a reason for hiding this comment

Uh oh!

koubaa commentedJan 15, 2020

Uh oh!

koubaaJan 15, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lostmsu commentedJan 16, 2020

Uh oh!

filmorJan 17, 2020

Choose a reason for hiding this comment

Uh oh!

filmorJan 17, 2020

Choose a reason for hiding this comment

Uh oh!

lostmsu commentedJan 30, 2020

Uh oh!

codecov-io commentedFeb 1, 2020• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lostmsu commentedFeb 13, 2020

Uh oh!

lostmsu commentedFeb 26, 2020

Uh oh!

Uh oh!

amos402 commentedFeb 28, 2020

Uh oh!

igiona commentedSep 16, 2021

Uh oh!

lostmsu commentedSep 16, 2021• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

igiona commentedSep 17, 2021

Uh oh!

lostmsu commentedSep 17, 2021• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

igiona commentedSep 19, 2021• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

lostmsu commentedSep 19, 2021

Uh oh!

Uh oh!

lostmsu commentedDec 19, 2019•
edited
Loading

lostmsuDec 22, 2019•
edited
Loading

codecov-io commentedFeb 1, 2020•
edited
Loading

lostmsu commentedSep 16, 2021•
edited
Loading

lostmsu commentedSep 17, 2021•
edited
Loading

igiona commentedSep 19, 2021•
edited
Loading