Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

#2240 Recursion error when doing reversed python operations on C# types#2327

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

Merged
lostmsu merged 5 commits intopythonnet:masterfromgertdreyer:bugfix/recursionError
Feb 26, 2024

Conversation

gertdreyer
Copy link
Contributor

What does this fix? Explain your changes.

Reverse operators were not implemented correctly for C# types which resulted in Recursion errors when embedding C# in Python. Changed the model binding process to add reverse operators for all applicable types. Added a flag to MethodBinder that signals the operation is a reversed operation and that the arguments to the operator needs to be reversed.

Added tests and an wrapper object for int without overloaded operator methods and a wrapper codec to test the reverse operations (and forward operator on the same type for safety).
...

Does this close any currently open issues?

This shouldresolve#2240.
...

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 toAUTHORS
  • Updated theCHANGELOG

@gertdreyergertdreyer changed the titleBugfix/recursion error #2240 Recursion error when doing reversed python operations on C# typesFeb 23, 2024
Copy link
Member

@lostmsulostmsu left a comment
edited
Loading

Choose a reason for hiding this comment

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

Can you explain what the issue was (e.g. what sequence of actions caused the error, and how codecs were a part of it) and how this fix addresses this scenario?

@gertdreyer
Copy link
ContributorAuthor

Changes made...

I traced it down to the nb_add, nb_mul, etc slots only having the forward operators defined/linked for C# types being used in Python, this somehow caused the RecursionError when any reverse operators are called as they are probably just pointing to random pointers.

py * cs callspy.__mul__(cs) which is returns not implemented for the cs type and codecs does not get called prior to themul call which results incs.__rmul__(py) being called which caused the recursion error. I do not believe that there are problems with the codec system but it is needed to test this due to being on both sides of the CLR boundary.

It seems there were only catered for forward operators when both are the different (with overloaded operators, i.e. OwnInt - Int64 and Int64 - OwnInt). Thus all the code to detect whether a call is a reverse operator tested based on calling and passed types. Thus when called with the same types it could never detect it was a reversed operation even if it were defined. This needed some changes to the MethodBinding and ModelBinding to ensure the reversed calls are linked to the right C# operators and called with the arguments swapped.

lostmsu reacted with thumbs up emoji

/// <returns>A Binding if successful. Otherwise null.</returns>
internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw)
internal Binding? Bind(BorrowedReference inst, BorrowedReference args, BorrowedReference kw, bool argsReversed = false)
Copy link
Member

Choose a reason for hiding this comment

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

Do you need to pass this parameter at all? Don't you have the field?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Cleaned it up a bit. I missed that... The one method is static though so we will need to still pass it there...

lostmsu reacted with thumbs up emoji
@lostmsulostmsu merged commit71ca063 intopythonnet:masterFeb 26, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@lostmsulostmsulostmsu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RecursionError when substracting System Decimal from Python Decimal
2 participants
@gertdreyer@lostmsu

[8]ページ先頭

©2009-2025 Movatter.jp