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

Python 3.11#1955

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
filmor merged 14 commits intopythonnet:masterfromfilmor:python-3.11
Nov 2, 2022
Merged

Python 3.11#1955

filmor merged 14 commits intopythonnet:masterfromfilmor:python-3.11
Nov 2, 2022

Conversation

filmor
Copy link
Member

  • Add Python 3.11 type offsets
  • Add Python 3.11 to metadata and workflows
  • Improve geninterop script to handle new case in 3.11
  • Fix offsets for 3.11

What does this implement/fix? Explain your changes.

...

Does this close any currently open issues?

...

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

Comment on lines +479 to +482
var slots = new[] {
new TypeSpec.Slot(TypeSlotID.tp_traverse, subtype_traverse),
new TypeSpec.Slot(TypeSlotID.tp_clear, subtype_clear)
};
Copy link
Member

@lostmsulostmsuOct 18, 2022
edited
Loading

Choose a reason for hiding this comment

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

This creates multiple sources of truth. Does editing post creation not work?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

It doesn't. Python 3.11 rejects creating a type that hasHAVE_GC set but not these slots. Rightly so :)

Copy link
Member

Choose a reason for hiding this comment

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

Can we have this array shared with the other place that used to use this function?

@filmor

This comment was marked as resolved.

@filmor

This comment was marked as resolved.

@lostmsu
Copy link
Member

I can take a look once 3.11 hits Anaconda.

@lostmsu
Copy link
Member

Re only clear ontp_dictoffset > 0. What is the scenario when it does not hold? I suspect the assert might be there to ensure something did not blow up earlier.

@filmor
Copy link
MemberAuthor

Re only clear ontp_dictoffset > 0. What is the scenario when it does not hold? I suspect the assert might be there to ensure something did not blow up earlier.

tp_dictoffset is only> 0 if there is actually atp_dict, which is apparently not the case anymore for a lot of classes in Python 3.11. Fixing this fixed almost all segfaults (which were probably just unhandled .NET exceptions in the end). The only one left is the derived class inRecursiveInheritance requiring a similar setup for the slots (i.e. up-front instead of via modification) as I did in the other case. Here, it complains abouttp_traverse.

@lostmsu
Copy link
Member

OK, just wanted to ensure you investigated. Any specific type combinations that turn out to be an issue to make a test?

@filmor
Copy link
MemberAuthor

The only issue left is fixing (and reenabling tests) forPy_SetPythonHome (which is actually deprecated since 3.11). I'm not quite sure what's going on here, but something seems to fail in the encoding as it assumes in the test that the path is\0xfffd, so Unicode Replacement Character.

@filmor
Copy link
MemberAuthor

The remaining issue seems to be precisely callingPy_SetPythonHome with an empty string, which seems to corrupt the underlying storage. I'll investigate a bit more, but in the worst case, we should probably just forbid it.

@lostmsu
Copy link
Member

LGTM, but I'll wait for the CI passing.

@filmor
Copy link
MemberAuthor

filmor commentedNov 1, 2022
edited
Loading

Wow, from my PoV this is actually a bug in Python itself:

https://github.com/python/cpython/blob/c0859743d9ad3bbd4c021200f4162cfeadc0c17a/Python/pathconfig.c#L255-L273

IfPy_SetPythonHome was called with a non-empty string and is subsequently called with an empty one, it will always runPyMem_RawFree but only actually reset the pointer in.home ifhas_value is set, so ifhome && home[0] (i.e. non-empty string).

I'll try to create a proper bug-report for this, but for now I will just forbid settingPythonHome to an empty string. All of the other property setters have the same issue, AFAICT.

The issue occurs in our test-suite, because we usually start up with an emptyPYTHONHOME, and then we try to update and later reset it. I don't know why this only occurs in Python 3.11, but the bug as such exists before that as well.

lostmsu reacted with eyes emoji

@filmorfilmorforce-pushed thepython-3.11 branch 2 times, most recently froma4ac4f2 tof1be5b6CompareNovember 2, 2022 07:30
@filmorfilmor marked this pull request as ready for reviewNovember 2, 2022 08:05
@filmorfilmor merged commit6968d01 intopythonnet:masterNov 2, 2022
@filmorfilmor deleted the python-3.11 branchNovember 2, 2022 08:06
@filmor
Copy link
MemberAuthor

I have investigated the issue further: It's indeed a regression in Python 3.11, as soon as it's fixed up-stream, I'll adjust our tests to always run again.

Martin-Molinero pushed a commit to Martin-Molinero/pythonnet that referenced this pull requestMar 15, 2024
Martin-Molinero added a commit to QuantConnect/pythonnet that referenced this pull requestMar 18, 2024
* Merge pull requestpythonnet#1955 from filmor/python-3.11Python 3.11* Minor fix for datetime tz conversion* Version bump to 2.0.29---------Co-authored-by: Benedikt Reinartz <filmor@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@lostmsulostmsulostmsu left review comments

@milosivanovicmilosivanovicmilosivanovic left review comments

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

Successfully merging this pull request may close these issues.

4 participants
@filmor@lostmsu@milosivanovic@br-sk

[8]ページ先頭

©2009-2025 Movatter.jp