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.8#1138

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 8 commits intomasterfrompython38
May 16, 2020
Merged

Python 3.8#1138

filmor merged 8 commits intomasterfrompython38
May 16, 2020

Conversation

filmor
Copy link
Member

@filmorfilmor commentedMay 10, 2020•
edited
Loading

Adds Python 3.8 to the CI and fixes the remaining issue by assigning__classcell__ the created class if it exists. This is required as for the case of subclassing a C# class in Python, we (currently) don't initialise the type viatype_new (== PyType_Type.tp_new) but instead generate it manually. The added code is done at the end oftype_new and is required from Python 3.8 onwards (see command below).

@filmor
Copy link
MemberAuthor

filmor commentedMay 10, 2020•
edited
Loading

The actual problem to fix is this (fromhttps://docs.python.org/3/reference/datamodel.html#creating-the-class-object):

CPython implementation detail: In CPython 3.6 and later, the__class__ cell is passed to the metaclass as a__classcell__ entry in the class namespace. If present, this must be propagated up to thetype.__new__ call in order for the class to be initialised correctly. Failing to do so will result in aRuntimeError in Python 3.8.

I'll look into it tomorrow.

@filmorfilmorforce-pushed thepython38 branch 2 times, most recently fromf3714a2 to82b8fdbCompareMay 14, 2020 23:12
@filmorfilmor marked this pull request as ready for reviewMay 14, 2020 23:13
@filmorfilmor requested a review fromlostmsuMay 14, 2020 23:13
If `PyCell_Set` is not called with a cell it will raise an exception,which is perfectly valid here.
@filmor
Copy link
MemberAuthor

Okay, so one bug left to fix: On Linux with .NET Core and Python 3.8 some of the embedding tests fail, will have a look.

@filmor
Copy link
MemberAuthor

@lostmsu Tests are now passing for all cases as far as I can see, can you have a look? I'll update the Changelog and then this is good to go.

@filmorfilmor linked an issueMay 15, 2020 that may beclosed by this pull request
@codecov-io
Copy link

codecov-io commentedMay 15, 2020•
edited
Loading

Codecov Report

Merging#1138 intomaster willnot change coverage.
The diff coverage isn/a.

Impacted file tree graph

@@           Coverage Diff           @@##           master    #1138   +/-   ##=======================================  Coverage   86.66%   86.66%           =======================================  Files           1        1             Lines         300      300           =======================================  Hits          260      260             Misses         40       40
FlagCoverage Δ
#setup_linux65.33% <ø> (ø)
#setup_windows72.00% <ø> (ø)

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last updated8f5ab0...ed2b7e8. Read thecomment docs.

Copy link
Member

@lostmsulostmsu left a comment

Choose a reason for hiding this comment

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

Apart from NITs looks fine. However, we should not remove public members fromRuntime in minor release.

- Use `BorrowedReference` where applicable- Readd `OperatingSystemName` and `MachineName` for now
@filmorfilmor merged commit782eff8 intomasterMay 16, 2020
@filmorfilmor deleted the python38 branchMay 16, 2020 18:21
Comment on lines 283 to 290
// Update the __classcell__ if it exists
IntPtr cell = Runtime.PyDict_GetItemString(cls_dict, "__classcell__");
if (cell != IntPtr.Zero)
{
Runtime.PyCell_Set(cell, py_type);
Runtime.PyDict_DelItemString(cls_dict, "__classcell__");
}

Copy link
Member

Choose a reason for hiding this comment

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

@filmor I can't find a test, that would cover this scenario, and I suspect there might be a problem with reference counting: if__classcell__ is present, its content is aPyType*, which might need to be DecRefed before being replaced withpy_type, andpy_type might need to be IncRefed before being put into the cell.

Also, I don't understand why__classcell__ is removed later. If it is removed, what is the purpose of updating its value?

Copy link
Member

Choose a reason for hiding this comment

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

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.

Finalise Python 3.8 support
3 participants
@filmor@codecov-io@lostmsu
[8]ページ先頭
©2009-2025 Movatter.jp