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

.NET objects returned as interface 'lose' their most-derived type identity #2213

Open
leepgent opened this issue Aug 7, 2023 · 8 comments
Open

Comments

@leepgent
Copy link

leepgent commented Aug 7, 2023

Environment

  • Pythonnet version: 3.0.1
  • Python version: 3.10.11
  • Operating System: Windows 11
  • .NET Runtime: .NET Framework 4.8.9167.0

Details

  • Describe what you were trying to get done.
    Return an interface derived from the one declared as a method or property's return type and be able to do RTTI to detect the type of derived interface returned. Do the same with a heterogeneous collection where the collection's type is declared as a collection of BaseInterface but the members are implementations of DerivedInterfaces.

  • What commands did you run to trigger this issue?
    Given some C# library code:

public interface IBaseInterface { }

public interface IDerivedInterfaceX : IBaseInterface  { }

public interface IDerivedInterfaceY : IBaseInterface  { }

public class ConcreteX : IDerivedInterfaceX { }

public class ConcreteY : IDerivedInterfaceY { }

public class Factory
{
    public IBaseInterface PolymorphicObject => new ConcreteX();
    public IEnumerable<IBaseInterface> HeterogenousPolymorphicCollection=> new IBaseInterface[] { new ConcreteY(), new ConcreteX() };
}

Python client code behaves differently than C# client code:

static void Main(string[] args)
{
    Factory f = new();
    IBaseInterface obj = f.PolymorphicObject;
    bool isDerived = obj is IDerivedInterfaceX; // true

    foreach (IBaseInterface obj2 in f.HeterogenousPolymorphicCollection)
    {
        bool isDerivedX = obj2 is IDerivedInterfaceX; // false then true
        bool isDerivedY = obj2 is IDerivedInterfaceY; // true then false
    }
}
f = Factory()

obj = f.PolymorphicObject  # Type is IBaseInterface FOREVER
isDerived = isinstance(obj, IDerivedInterfaceX)  # Always False
isSubclass = issubclass(type(obj), IDerivedInterfaceX)  # Always False

for obj2 in f.HeterogenousPolymorphicCollection:  # Type is IBaseInterface FOREVER
    isDerivedX = isinstance(obj2, IDerivedInterfaceX)  # ALWAYS FALSE
    isDerivedY = isinstance(obj2, IDerivedInterfaceY)  # ALWAYS FALSE

We know it's possible to "cheat" by testing obj.__implementation__'s type but we'd rather keep everything as interfacey as possible :)

We know the official solution is to attempt to upcast the returned objects but of course this will throw an exception if it's not possible -- we'd like to examine the type returned to ensure no exception before we upcast.

Is there a nice, pretty solution?

@lostmsu
Copy link
Member

lostmsu commented Aug 7, 2023

Maybe we should fix the isinstance and issubclass to return True, but can't you just do the following?

def is_clr_instance(obj, T):
  if hasattr(obj, '__implementation__'):
    return isinstance(obj.__implementation__, T)
  else:
    return isinstance(obj, T)

and a similar one for is_clr_subclass

@filmor
Copy link
Member

filmor commented Aug 7, 2023

We can't adjust isinstance and issubclass (which is correct anyhow, one would need to adjust type(obj)) without also going back to the "old" behaviour of trying to resolve all fields, properties and methods dynamically. Otherwise, if isinstance(obj, SomeClass): obj.PropertyDefinedInSomeClass would fail.

@lostmsu
Copy link
Member

lostmsu commented Aug 8, 2023

@filmor won't fix then? The simple method above is a descent workaround.

@leepgent
Copy link
Author

leepgent commented Aug 8, 2023

going back to the "old" behaviour of trying to resolve all fields, properties and methods dynamically

I'll take any mechanism which just tells me if an particular interface is implemented dynamically so we can upcast to get to the subclass fields/props/methods 'statically'' without having to check __implementation__ (which seems quite un-OOP when you're trying very hard to hide the "concrete implementation" types!

@filmor
Copy link
Member

filmor commented Aug 9, 2023

It's quite un-OOP to just assume that a particular interface might be implemented. If you want to use a property, make sure that the returned interface has that property. You'd have to do that in C# as well. The main issue here is that isinstance, issubclass and type have different semantics from their C# counterparts. We could add helper functions like the one lostmsu gave (maybe also a try_cast).

@leepgent
Copy link
Author

leepgent commented Aug 9, 2023

If you want to use a property, make sure that the returned interface has that property. You'd have to do that in C# as well.

This isn't always going to be possible though - hierarchies of interfaces do exist (like the one I've "inherited", ho ho ho). I absolutely agree in the real world this problem is the result of a terrible design but it isn't going to be practical or possible to flatten them all out and add N hundred methods returning N hundred possible interfaces/having N homogenous Collections when they all share a common interface ancestor (or crunching Z hundred members down into one mega-interface). In C# at least you can test the returned interface to see if it's actually a subinterface and then cast it before trying access the property (or do it in one go with is).

I don't think it's unreasable to ask a object of IBase if it is actually IDerivedFromBase and there's no neat way to do this yet in Python (and naturally I absolutely agree with with policy of restricting access to IDerivedFromBase members until it's been casted).

I totally agree that modifying isinstance et al would be sketchy and we should avoid it. In the meantime, I've been experimenting and there is a solution which does't rely on looking behind the curtain at __implementation__: get .NET to do the query:

def try_cast(T, obj):
    return T(obj) if clr.GetClrType(T).IsInstanceOfType(obj) else None


f = Factory()

obj = f.PolymorphicObject  # Defined as returns IBaseInterface
subinterfaceIX = clr.GetClrType(IDerivedInterfaceX).IsInstanceOfType(obj)  # <IDerivedInterfaceX>, same a C# code
obj = IDerivedInterfaceX(obj)  # Happy Days
#obj = IDerivedInterfaceY(obj)  # Crashy Days

for obj2 in f.HeterogenousPolymophicCollection:  # Defined as IEnumerable<IBaseInterface>
    derivedIX = try_cast(IDerivedInterfaceX, obj2)  # None then <IDerivedInterfaceX>, same as C# code
    derivedIY = try_cast(IDerivedInterfaceY, obj2)  # <IDerivedInterfaceY>then None, same as C# code

I don't doubt there's a neater way to do this and/or a having a package-provided helper function but for now we're sorted - thanks.

@filmor
Copy link
Member

filmor commented Aug 9, 2023

Yeah, I think I'll prepare a PR for adding functions for this. We should have probably added these right when the change in behaviour happened.

@lostmsu
Copy link
Member

lostmsu commented Aug 9, 2023

@filmor basically, .NET's is and as functions directly on clr module?

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

3 participants