fix: Python-3.12 compatibility

[sizmailov]

Enable dynamic attributes for pybind11_static_property for compatility with cpython-3.12

Description

The problem is that property-derived classes now use __doc__ attribute, which in turn requires to enable "dynamic attributes". All that seems like a pessimisation which is not welcome here. I'm submitting the PR to demonstrate a possible solution if the compatibility issue will be solved on pybind side.

Sorry for all the trouble. I thought I'd been fixing cpython, not breaking pybind. 😞

Suggested changelog entry:

Enable dynamic attributes for `pybind11_static_property` for Python-3.12

Closes #4115

[rwgk]

Hi level questions, these may be naive or infeasible:

Sorry for all the trouble. I thought I'd been fixing cpython, not breaking pybind.

• Could the change in cpython still be undone or done differently? -- Background thoughts: In the most general case, py::dynamic_attr() / enable_dynamic_attributes() opens up pitfalls, therefore I look at that feature as a last resort or escape hatch. But then again, this PR only changes what's more-or-less a pybind11-internal type (IIUC; I haven't actually worked on that part of pybind11).

• Is this PR in any way related to Py_TPFLAGS_MANAGED_DICT? How does it play with PR Undo attr.h, detail/class.h changes made under PR #3923 #4142? (I'm still unclear what's better, dropping or merging the PR. What's your advice?)

• Is there a reasonable and meaningful way to set up a pybind11 CI job to test against cpython 3.12? At the moment we're only testing with cpython 3.11, using something pre-packed (I don't know how it works under the hood, but I know with had/have to wait for b1, b2, rc1, rc2 releases). -- I.e. what I have in mind is a CI job that first builds Python from sources, then uses that to test pybind11.

[sizmailov]

I believe python method resolution rules leave no alternative to dynamics attributes to correctly implement the property.__init__ and preserve __doc__ in subclasses. At least in a non-intrusive way.

There are more troubles for __doc__ specifically because each derived class (implicitly) defines its own __doc__ which takes precedence over the instance's __doc__ defined in its parent class.

The feature freeze for Python 3.12 should be around May 2023, so I believe it can be undone or done differently.

Another possible solution would be to avoid subclassing built-in property and implement descriptor protocol from scratch, getting all the performance we want and correctness at the same time. But this approach would add 100+ lines to pybind source and potentially breaks compatibility. Do you think this would be worth a try?

I read through the lined issues/PR, and I believe this issue is not related to Py_TPFLAGS_MANAGED_DICT. After all, we have just base class here, so this should not interfere.

WRT CI, I think someone just adds more images to the list once a new python version gets released. I think having an opportunity to test against an upstream python version is a nice feature. Although such CI builds will not be reproducible and prone to spurious errors due to rapid changes in CPython.

[rwgk]

Another possible solution would be to avoid subclassing built-in property and implement descriptor protocol from scratch, getting all the performance we want and correctness at the same time. But this approach would add 100+ lines to pybind source and potentially breaks compatibility. Do you think this would be worth a try?

First thought: it doesn't seem attractive, because it sounds like complex functionality is duplicated, with small tweaks. Is that a fair way to look at it? — If yes, we'll have the potential for divergence over time, with all the confusion that usually arises from such a situation.

I read through the lined issues/PR, and I believe this issue is not related to Py_TPFLAGS_MANAGED_DICT. After all, we have just base class here, so this should not interfere.

Thanks for the explanation!

WRT CI, I think someone just adds more images to the list once a new python version gets released. I think having an opportunity to test against an upstream python version is a nice feature. Although such CI builds will not be reproducible and prone to spurious errors due to rapid changes in CPython.

Yeah, we'd need some way to easily pin on a CPython commit, but that's probably easy to achieve. (I'm itching to try ... but someone must have done something similar already ...)

Coming back to this PR, it's tiny, and if it helps testing with pybind11 while 3.12 is still fluid, I think we should merge, but maybe with a special comment? E.g. // PRE 3.12 FEATURE FREEZE. PLEASE REVIEW AFTER FREEZE.

[sizmailov]

I totally agree, re-implementing the property is not a picnic. Although, the maintenance should be rather low. I don't think Python would break C API or descriptor protocol too often. Overall, yeah, this option is not too appealing.

I tried a no-brainer python build from source on CI, but it failed at pybind linking stage. You are right, pinning a version is not a problem. I'll submit a PR if I squeeze something meaningful out of it.

[rwgk]

I tried a no-brainer python build from source on CI, but it failed at pybind linking stage. You are right, pinning a version is not a problem. I'll submit a PR if I squeeze something meaningful out of it.

JIC you find this useful: https://github.com/rwgk/pybind11_scons/blob/8c15a4c54a010beb650084c801a5c48464e930b7/SConscript#L155

(That small SConscript has pretty much all tricks required in one easy view, but Linux only.)

[rwgk]

I forgot to mention, I find -DCMAKE_VERBOSE_MAKEFILE=ON (config option) super useful to be sure about what is actually happening.

[rwgk]

@henryiii @Skylion007 Do you want to take a look?

[rwgk]

No response from @henryiii @Skylion007 for 9 days, I assume this PR is fine with them.

In the meantime 311rc2 was released.

I will try to:

• rebase this PR on current master
• add the python dev label (which will activate testing with 311rc2, to get two birds with one stone ... assuming it goes well)
• then merge

Hang on.

[henryiii]

Looks okay if tests pass.

[henryiii]

Fine by me if it passes 3.12 tests.

[rwgk]

Fine by me if it passes 3.12 tests.

Thanks!

[tacaswell]

I can confirm that this fixes the problem for me.

[henryiii]

We'll try to get a release out in the "near" future. "Near" is a fluid term, though.

[Skylion007]

@henryiii @rwgk Do we need what commit in CPython necessitated this change? Want to make sure this expected behavior as opposed to a 3.12 bug.

[Skylion007]

Ah, for future reference it's python/cpython#23205