Allow NULL value in pybind11_meta_setattro

[bstaletic]

Description

Python says we have to handle NULL values in pybind11_meta_setattro as "delete property".

https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_setattro

However, pybind11 currently doesn't allow users to delete non-static properties. I'm not really sure what's the right course of action, this PR is just the first thing that worked, in the sense of not segfaulting as in #2009.

Fixes #2009

Suggested changelog entry:

Allows deleting of static properties.

[bstaletic]

So this check fails because:

>>> import pybind11_tests
>>> m = pybind11_tests.methods_and_attributes
>>> m.TestPropertiesOverride.def_readonly_static
99
>>> del m.TestPropertiesOverride.def_readonly_static
>>> hasattr(m.TestPropertiesOverride, 'def_readonly_static')
True
>>> m.TestPropertiesOverride.def_readonly_static
1
>>> del m.TestPropertiesOverride.def_readonly_static
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: def_readonly_static

Can anyone make sense of it?

[YannickJadoul]

The latter value, 1, seems to come from m.TestProperties, a base class of m.TestPropertiesOverride?

[bstaletic]

That's right.

>>> m = pybind11_tests.methods_and_attributes
>>> del m.TestPropertiesOverride.def_readonly_static
>>> m.TestPropertiesOverride.def_readonly_static
1
>>> del m.TestProperties.def_readonly_static
>>> m.TestPropertiesOverride.def_readonly_static
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: type object 'pybind11_tests.methods_and_attributes.TestProperti' has no attribute 'def_readonly_static'

This makes me even less sure if this is the right course of action.

[YannickJadoul]

Can we see/mimic how this would react in Python?

[YannickJadoul]

Just tried this out:

>>> class Base:
...     answer = 6 * 9
... 
>>> class Derived(Base):
...     answer = 42
... 
>>> Derived.answer
42
>>> del Derived.answer
>>> Derived.answer
54
>>> del Derived.answer
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: answer
>>> del Base.answer
>>> Derived.answer
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: type object 'Derived' has no attribute 'answer'
>>> 

So even without any properties, static attributes in pure Python would match our behavior, no?

So ... not a bug, just counter-intuitive Python behaviour?

[YannickJadoul]

Unless there's a reason to match non-static properties (or something that needs to be kept alive or so), I'm fine with this. It matches pure Python behavior, and as far as I know, that's what we most of the time aim for.

[henryiii]

2.6.1 or 2.7?

[henryiii]

That was fast... :)

[YannickJadoul]

That was fast... :)

Hahahaha, I was just thinking I missed a PR or a few somewhere, and was already looking around ;-)
Do we need all PRs to be added to a milestone? I still haven't heard how we're going to manage versions.

[rwgk]

My vote is to include this one in 2.6.1.

[YannickJadoul]

Merged! Good catch, @fried, and thanks for fixing, @bstaletic!