Repository navigation
Python 3.8#1138
Conversation
Sorry, something went wrong.
|
The actual problem to fix is this (from https://docs.python.org/3/reference/datamodel.html#creating-the-class-object):
I'll look into it tomorrow. |
Sorry, something went wrong.
If `PyCell_Set` is not called with a cell it will raise an exception, which is perfectly valid here.
|
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. |
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
Codecov Report
@@ Coverage Diff @@
## master #1138 +/- ##
=======================================
Coverage 86.66% 86.66%
=======================================
Files 1 1
Lines 300 300
=======================================
Hits 260 260
Misses 40 40
Continue to review full report at Codecov.
|
Sorry, something went wrong.
lostmsu
left a comment
There was a problem hiding this comment.
Apart from NITs looks fine. However, we should not remove public members from Runtime in minor release.
Sorry, something went wrong.
- Use `BorrowedReference` where applicable - Readd `OperatingSystemName` and `MachineName` for now
| // 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__"); | ||
| } | ||
|
|
There was a problem hiding this comment.
@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 a PyType*, which might need to be DecRefed before being replaced with py_type, and py_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?
Sorry, something went wrong.
There was a problem hiding this comment.
NVM, I found the code to be replica of https://github.com/python/cpython/blob/145bf269df3530176f6ebeab1324890ef7070bf8/Objects/typeobject.c#L2844
Sorry, something went wrong.
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 via type_new (== PyType_Type.tp_new) but instead generate it manually. The added code is done at the end of type_new and is required from Python 3.8 onwards (see command below).