PyCXX Python-C++ Connection / Patches / #28 Supported Py_LIMITED

Barry Alan Scott - 2017-11-18

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:

Pavel Ostyakov - 2017-11-18

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:

Barry Alan Scott - 2017-11-27

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:

Pavel Ostyakov - 2017-11-28

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:

Barry Alan Scott - 2017-11-29

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?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Pavel Ostyakov - 2017-11-29

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:

Barry Alan Scott - 2017-12-03

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?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Pavel Ostyakov - 2017-12-06

No, the behaviour is not different for users. It's just the details of internal implementation

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Barry Alan Scott - 2018-01-01

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:

Andrey Saitgalin - 2018-04-13

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

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Barry Alan Scott - 2018-04-15

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:

Andrey Saitgalin - 2018-05-23

Hello, Barry! Just a friendly reminder :)

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Andrey Saitgalin - 2018-06-02

Hello, Barry! Any news here?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Barry Alan Scott - 2018-06-04

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:

Barry Alan Scott - 2018-06-04

status: open --> pending

assigned_to: Barry Alan Scott

Group: -->
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Andrey Saitgalin - 2018-06-04

Thanks a lot Barry! I will test all changes in our project and send a reply

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Barry Alan Scott - 2018-07-31

Andrey can I have the test report please? Its been more then a month.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Andrey Saitgalin - 2018-08-04

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:

#ifndef Py_LIMITED_API PyAPI_DATA(const char *) _Py_PackageContext; #endif

Kind regards,
Andrey.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Barry Alan Scott - 2018-08-08

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:
- Andrey Saitgalin - 2018-08-08
  
  Hello Barry!
  
  Thanks!
  
  Andrey.
  Barry Alan Scott barry-scott@users.sourceforge.net schrieb am Mi. 8. Aug.
  2018 um 12:26:
  
  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.
  
  [patches:#28] https://sourceforge.net/p/cxx/patches/28/ Supported
  Py_LIMITED_API*
  
  Status: pending
  Group:
  Created: Sat Nov 11, 2017 07:27 PM UTC by Pavel Ostyakov
  Last Updated: Sat Aug 04, 2018 10:39 AM UTC
  
  Owner: Barry Alan Scott
  Attachments:
  
  limited_api.patch
  https://sourceforge.net/p/cxx/patches/28/attachment/limited_api.patch
  (40.2 kB; text/x-patch)
  
  This patch provides a way to support stable Python ABI:
  https://www.python.org/dev/peps/pep-0384/
  
  Sent from sourceforge.net because you indicated interest in
  https://sourceforge.net/p/cxx/patches/28/
  
  To unsubscribe from further messages, please visit
  https://sourceforge.net/auth/subscriptions/
  
  Related
  
  Patches: #28
  
  alternate
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Supported Py_LIMITED_API

Group

Searches

Help

#28 Supported Py_LIMITED_API

Related

Discussion

commited as r394 and r395.

Related