Thank you very much for the patch this is a feature that I want PyCXX to support.
I have commited r386 to update the build-all.sh and setup_makefile.py to support testing this feature. running ./build-all.sh will test pycxx against all installed python versions.
I see this error with python 3.6 and will work on that.
./CXX/Python3/ExtensionType.hxx:313:38: error: ‘PyType_GetSlot’ was not declared in this scope
((freefunc)PyType_GetSlot( _self->ob_type, Py_tp_free ))( _self );
~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./CXX/Python3/ExtensionType.hxx:313:38: note: suggested alternative: ‘PyType_Slot’
Barry
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
You should define Py_LIMITED_API version 3.4 since limited abi for Python versions 3.3 or less does not support calling tp_dealloc: https://bugs.python.org/issue17162
So, just define Py_LIMITED_API as 0x03040000
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Yep, I've added the ability to test with different values of Py_LIMITED_API and defaulted to 0x0304000 and added a error to say that it most be 3.4 or later.
I have been busy and need to get back to this.
I need to understand why you have reordered/added calls in some of the patch. For example to readyType() in ExtensionOldType.hxx. Either the call is not needed or being missing is a bug.
As I say I need to find the time to get back into this.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
In case of limited API, we should call PyType_FromSpec to declare a new type. This logic is implemented in readyType(), so, we call it in ExtensionOldType because users don't call readyType() when defining old-type classes using PyCXX
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
It is protected by this #ifdef because we don't need to call PyType_Ready when declaring old-type classes. However, with Py_LIMITED_API, we should dynamically allocate new type because there are no another way to declare a new type except of calling PyType_FromSpec. The best way to do it is calling readyType() in initialization.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I'm working thought the Py_LIMITED_API bit by bit getting my head around it.
Where possible I want to have a single imlpementation of a feature by using the limited API only.
I'm not sure about how far I will get attempting this.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
How the things are going with this patch? This would be very useful for us, we are already using it in production libraries but it is not yet in upstream and there is no progress here. We reviewed this patch very carefully in three pairs of eyes and are really looking forward to seeing this patch accepted! :)
Thanks much,
Andrey
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Thaks for the reminder. This dropped off my todo list.
So far I have researched the API and found out why your patch does what it does.
I recall finding some gaps in the patch that I started fixing.
I wil try and find the time to get this committed soon.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I have committed r388 with a modified version of your patch.
Please test in your environment and let me know is there are any problems.
I found that readyType() must be called at the end of the init_type() functions.
I have no idea why this was not neccesary in the !defined(Py_LIMITED_API) case.
All tests pass for all python from 2.7, 3.4 to 3.7. With limit api for 3.4 to 3.7.
There are new build scripts to run the build&test for limited and unlimited API
cases.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
As I can see _PyPackageContext is not under Py_LIMITED_API ifdef now, isn't it a mistake? In Python 3 sources it is not available in public limited api:
Thanks for the review.
1. I have put Complex back in, there are some parts of that class that need to be protected with Py_LIMITED_API that use Py_complex.
2. I missed only one place that PyPackageCOntext was not under Py_LIMITED_API, fixed.
commited as r394 and r395.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Thanks for the review.
1. I have put Complex back in, there are some parts of that class that
need to be protected with Py_LIMITED_API that use Py_complex.
2. I missed only one place that PyPackageCOntext was not under
Py_LIMITED_API, fixed.
Thank you very much for the patch this is a feature that I want PyCXX to support.
I have commited r386 to update the build-all.sh and setup_makefile.py to support testing this feature. running ./build-all.sh will test pycxx against all installed python versions.
I see this error with python 3.6 and will work on that.
./CXX/Python3/ExtensionType.hxx:313:38: error: ‘PyType_GetSlot’ was not declared in this scope
((freefunc)PyType_GetSlot( _self->ob_type, Py_tp_free ))( _self );
~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./CXX/Python3/ExtensionType.hxx:313:38: note: suggested alternative: ‘PyType_Slot’
Barry
You should define Py_LIMITED_API version 3.4 since limited abi for Python versions 3.3 or less does not support calling tp_dealloc: https://bugs.python.org/issue17162
So, just define Py_LIMITED_API as 0x03040000
Yep, I've added the ability to test with different values of Py_LIMITED_API and defaulted to 0x0304000 and added a error to say that it most be 3.4 or later.
I have been busy and need to get back to this.
I need to understand why you have reordered/added calls in some of the patch. For example to readyType() in ExtensionOldType.hxx. Either the call is not needed or being missing is a bug.
As I say I need to find the time to get back into this.
In case of limited API, we should call PyType_FromSpec to declare a new type. This logic is implemented in readyType(), so, we call it in ExtensionOldType because users don't call readyType() when defining old-type classes using PyCXX
I'd like to understand what is goig wrong here.
Why is that protected by an #ifdef Py_LIMITED_API?
How do I demonstrate that it is a bug without the change?
It is protected by this #ifdef because we don't need to call PyType_Ready when declaring old-type classes. However, with Py_LIMITED_API, we should dynamically allocate new type because there are no another way to declare a new type except of calling PyType_FromSpec. The best way to do it is calling readyType() in initialization.
But that means that the behviour is different it Py_LIMITED_API is defined that is surely a bad thing to do?
Are you saying that new-style classes cannot be defined with the Py_LIMITED_API?
No, the behaviour is not different for users. It's just the details of internal implementation
I'm working thought the Py_LIMITED_API bit by bit getting my head around it.
Where possible I want to have a single imlpementation of a feature by using the limited API only.
I'm not sure about how far I will get attempting this.
Hello, Barry!
How the things are going with this patch? This would be very useful for us, we are already using it in production libraries but it is not yet in upstream and there is no progress here. We reviewed this patch very carefully in three pairs of eyes and are really looking forward to seeing this patch accepted! :)
Thanks much,
Andrey
Thaks for the reminder. This dropped off my todo list.
So far I have researched the API and found out why your patch does what it does.
I recall finding some gaps in the patch that I started fixing.
I wil try and find the time to get this committed soon.
Hello, Barry! Just a friendly reminder :)
Hello, Barry! Any news here?
I have committed r388 with a modified version of your patch.
Please test in your environment and let me know is there are any problems.
I found that readyType() must be called at the end of the init_type() functions.
I have no idea why this was not neccesary in the !defined(Py_LIMITED_API) case.
All tests pass for all python from 2.7, 3.4 to 3.7. With limit api for 3.4 to 3.7.
There are new build scripts to run the build&test for limited and unlimited API
cases.
Thanks a lot Barry! I will test all changes in our project and send a reply
Andrey can I have the test report please? Its been more then a month.
Hello, Barry!
Sorry for such a long answer. I carefully compared PyCXX trunk and our version, run tests and everything seems fine in our codebase! Thanks!
I just want to comment some places in code which from my point of view could be handled more carefully.
https://sourceforge.net/p/cxx/code/HEAD/tree/trunk/CXX/CXX/Python3/Objects.hxx#l970
Why have you brought Complex in Python 3 under Py_LIMITED_API ifdef? Complex numbers are available in limited api as far as I can see it in Python 3 source code. For example, PyComplex_FromDoubles function is available and could be used.
As I can see
_PyPackageContext
is not under Py_LIMITED_API ifdef now, isn't it a mistake? In Python 3 sources it is not available in public limited api:Kind regards,
Andrey.
Thanks for the review.
1. I have put Complex back in, there are some parts of that class that need to be protected with Py_LIMITED_API that use Py_complex.
2. I missed only one place that PyPackageCOntext was not under Py_LIMITED_API, fixed.
commited as r394 and r395.
Hello Barry!
Thanks!
Andrey.
Barry Alan Scott barry-scott@users.sourceforge.net schrieb am Mi. 8. Aug.
2018 um 12:26:
Related
Patches: #28