Skip to content

Improved Type Interop with CLR #2055

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

Closed
wants to merge 4 commits into from
Closed

Conversation

eirannejad
Copy link
Contributor

@eirannejad eirannejad commented Dec 19, 2022

What does this implement/fix? Explain your changes.

This PR addresses these issues:

  • Current pythonnet does not allow subclassing from CLR Abstract classes
  • Running python script with types containing __namespace__, more than once, would throw Type Exists exception
  • Calling super().VirtualMethod() inside a python-derived virtual method, would result in an infinite loop

Due to the nature of the changes, this is a single PR instead of multiple smaller ones.

After PR merge:

  • Python types can derive from CLR abstract classes. As before, abstract methods that are not implemented on the python type, will dynamically throw NotImplemented exception.
  • Specifying __namespace__ is not required to created a ClassDerived anymore. All python derived classes are now represented by ClassDerived in managed memory.
  • ClassDerived now caches generated types based on namespace, type name, and chain of base classes. This avoids regenerating types or throwing Type Already Exists exceptions. A new format for generated type names include full name of base class and interfaces in the typename. The naming format still ends in the python type name to be backward compatible and passes the unit tests
    • Example: Full name of derived clr type for python type named SubClass, deriving from BaseClass and implementing BaseInterface1 and BaseInterface2:
      Python.Runtime.Dynamic.BaseClass__BaseInterface1__BaseInterface2__main__SubClass
  • Support for one base class and multiple interfaces (Merged some ideas and code from 1783 Implement Interface And Inherit Class #2028 - Potential merge conflicts)
  • Derived class can call chosen constructors using super().__init__() pattern.
class SubClass(BaseClassA):
   def __init__(self, v):
       super().__init__(v)
  • Derived class can call base virtual methods using super().method() pattern.
class SubClass(BaseClassA):
    def DoWorkWith(self, value):
        r = super().DoWorkWith(value) + 10
        return r
  • Improved virtual method and getter/setter routing. Custom attributes (OriginalMethod and RedirectedMethod) are now used to mark the original and redirected virtual methods. The method name format for OriginalMethod is changed to $"_BASEVIRTUAL__{name}" so it can handle calling original methods on base classes that does not match the name of current class. Previously this was not working.
  • Other misc refactoring and improvements. No change in behavior.

Does this close any currently open issues?

Potentially (no tests have been done to ensure these issues are resolved)

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
  • Ensure you have signed the .NET Foundation CLA
  • Add yourself to AUTHORS
  • Updated the CHANGELOG

@eirannejad eirannejad marked this pull request as ready for review December 30, 2022 21:29
@eirannejad
Copy link
Contributor Author

Hello. Would anyone help me get some information on the error on these test fails? I can not run the dotnet test --runtime any-x64 --logger "console;verbosity=detailed" src/embed_tests/ test on my macOS machine as it throws lots of exceptions. Not sure what sort of environment it needs for testing locally

@filmor
Copy link
Member

filmor commented Jan 9, 2023

If it fails to run locally, check the output. You are probably missing the PYTHONNET_PYDLL export. For me, it actually runs through on .NET Core but is extremely slow on Mono. That's why the respective CI jobs are timing out.

@lostmsu
Copy link
Member

lostmsu commented Jan 20, 2023

Please, rebase on master if you are still willing to work on getting this PR in.

@eirannejad
Copy link
Contributor Author

@lostmsu Sure thing. Working on getting master merged in

@mhsmith
Copy link

mhsmith commented Jul 11, 2023

As part of this work, it would be good for Python subclassing of CLR types to be properly documented (#674 (comment)).

@eirannejad
Copy link
Contributor Author

Closing this PR as it is too old. I have merged the 3.0.3 master into my fork so I can create new updated PRs

@eirannejad eirannejad closed this Dec 8, 2023
@eirannejad eirannejad deleted the pr/newtypes branch December 20, 2023 22:13
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

Successfully merging this pull request may close these issues.

4 participants