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

Allow decoders to decode Python types derived from primitives #2230

Open
Bluubb opened this issue Sep 1, 2023 · 7 comments
Open

Allow decoders to decode Python types derived from primitives #2230

Bluubb opened this issue Sep 1, 2023 · 7 comments

Comments

@Bluubb
Copy link

Bluubb commented Sep 1, 2023

Environment

  • Pythonnet version: 3.0.2
  • Python version: not relevant for this issue
  • Operating System: not relevant for this issue
  • .NET Runtime: not relevant for this issue

Details

  • We have used the preview version in the past and wrapped it so that we are able to update the package and change against interfaces for future releases. One part of it is converting numpy types. By default such types are not supported anymore in the release version and it seems to be recommended to use codecs instead. I registered a codec (same as pandanet) but PyObject.As<double>() does not convert PyType = <class 'numpy.int32'> with the help of the codec.
    By debugging it never hits the PyObjectConversions.TryDecode because double it is not equal to "object type" and not one of the "DecodableByUserTypes".

    TODO

to reproduce:

    import numpy as np
    numpyArray = np.array([1, 2])
// C# part
var value = numpyArray.GetItem(i)
var valueAsT = value.As<double>(); // fails

Whish:

  • solution to handle such a case i.e. by configuring hooks or use the existing codec approach to convert before it hits the usual ToPrimitive(...) function which fails.
@lostmsu
Copy link
Member

lostmsu commented Sep 1, 2023

Related #1887

@lostmsu
Copy link
Member

lostmsu commented Sep 1, 2023

There are few workarounds for this:

  1. Explicitly call .item(). E.g. Math.Abs(np.int64(42)) --> fail; Math.Abs(np.int64(42).item()) --> OK
  2. It seems that your GetItem is a custom method. It could in principle perform the conversion and return double instead of PyObject. This is the approach I have in https://www.nuget.org/packages/LostTech.NumPy/

@lostmsu
Copy link
Member

lostmsu commented Sep 1, 2023

As for the suggestion itself, my major concern about adding codecs for primitive types is that they will be invoked on any attempt to convert to a .NET primitive, which in many cases will have a noticeable performance impact.

@Bluubb
Copy link
Author

Bluubb commented Sep 6, 2023

There are few workarounds for this:

  1. Explicitly call .item(). E.g. Math.Abs(np.int64(42)) --> fail; Math.Abs(np.int64(42).item()) --> OK
  2. It seems that your GetItem is a custom method. It could in principle perform the conversion and return double instead of PyObject. This is the approach I have in https://www.nuget.org/packages/LostTech.NumPy/
  1. Seems to be a change to all existing usages -> breaking, without changing all consumers
  2. PyObject.GetItem(int) is an existing function in this pythonnet library

@Bluubb
Copy link
Author

Bluubb commented Sep 6, 2023

As for the suggestion itself, my major concern about adding codecs for primitive types is that they will be invoked on any attempt to convert to a .NET primitive, which in many cases will have a noticeable performance impact.

How about adding a flag:

if(IsPrimitiveCodecsEnabled) // no noticable performance issue with single bool check
{
    var isSucess = TryDecode(...) // via codec
    if(isSuccess){return;}
}
ToPrimitive(...)
return;

@lostmsu
Copy link
Member

lostmsu commented Sep 7, 2023

We will not be adding flags, as every flag adds a whole new dimension to the testing matrix.

@lostmsu
Copy link
Member

lostmsu commented Sep 7, 2023

@Bluubb can you tell more about your use case? Is there a reason you can not make a function to replace As<double>()?

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

No branches or pull requests

2 participants