Make handle and related classes const correct.

[ghost]

This gives handle classes a typical pointer semantics with respects to
constness.

It seems that now only buffer class that originally triggered issue #35 is left
to be fixed. But this should be trivial once ownership of Py_buffer is moved to
buffer_info #45.

[wjakob]

I'm not sure I agree with all of the changes -- take for instance begin() and end(), which now return a normal (non-const iterator). This could for instance be used to modify entries of a const pybind11::list, which doesn't seem right -- can you clarify?

[ghost]

In general I wanted handle and derived classes to behave in the same way as pointers do with respect to constness, i.e., const is not transitive. For example:

  const shared_ptr<vector<int>> x = ..., y = ...;
  // Changing pointer is not allowed:
  x = y;
  // Changing pointee is allowed:
  x->clear();
  x->push_back(1);

Thus, behaviour of begin() and end() is intentional.

[adler-j]

While you are technically correct in regards to pointers, we cannot generalize this to handles. For example, we cannot make the equivalent to:

shared_ptr<const vector<int>> x = ..., y = ...;
// Changing pointer is allowed:
x = y;
// Changing pointee is not allowed:
x->clear();
x->push_back(1);

and this is what users general expect from const (I know I would).

[ghost]

I would give the following arguments for proposed behaviour:

• The handle have pointer like behaviour in other aspects, so for general
  consistency with C++ it would be reasonable to expect it to behave like one
  with respects to constness. Making const transitive may mislead someone into
  thinking that those classes have value like semantics (with respect to
  PyObject they point to), where in fact they don't have it.
• There is no concept of const in Python, so division between const and
  non-const methods will be somewhat arbitrary and will not have usual C++
  meaning. Take for example defaultdict from collections. It inserts a new item
  into dictionary if key is not found during lookup. Thus, if key lookup were
  const it would still modify the Python dictionary.

d = defaultdict(int)
print(d) # defaultdict(<class 'int'>, {})
d[0]
print(d) # defaultdict(<class 'int'>, {0: 0})

• Usefulness of const is limited considerably because how easy it is to drop
  it, even by accident. Consider following example:

void modify(py::object x) {
  x["key"] = value;
}

const py::object x {...}
// Not allowed:
//x["key"] = value;
// But following is allowed and modifies the same Python object:
modify(x);

[adler-j]

I would give the following arguments for proposed behaviour:

Very well thought through arguments, this would be my response:

The handle have pointer like behaviour in other aspects, so for general consistency with C++ it would be reasonable to expect it to behave like one with respects to constness. Making const transitive may mislead someone into thinking that those classes have value like semantics (with respect to PyObject they point to), where in fact they don't have it.

Not quite sure I agree here. Handles have methods that directly access the pointee, for example, with pointers one would need to do:

shared_ptr<vector<int>> x = ...;
x->operator[](4) = 5

while with handles we would do

py:array x = ...;
x[4] = 5

thus in some sense the handle "is" the object. In the other sense (owning data), you are correct.

I'd actually liken handles the most with references, and references use const in the more strict sense.

int x = ...;
int& y = x;
y = 5; // Also updates x

const int& z = x;
z = 5; // Not possibly since z is declared const

There is no concept of const in Python, so division between const and
non-const methods will be somewhat arbitrary and will not have usual C++
meaning. Take for example defaultdict from collections. It inserts a new item
into dictionary if key is not found during lookup. Thus, if key lookup were
const it would still modify the Python dictionary.

Easily your strongest argument and an obvious issue. My approach would be to assume getters are const and setters are not. This is for example how PyCXX does it.

Related issue: your example would currently be problematic given the current code since PyObject_GetItem is called only in the implicit conversion to object.

py::map map = ...;
map[0]; // Returns an py::accessor, does not call PyObject_GetItem
auto x = map[0]; // Returns a py::accessor, does not call PyObject_GetItem
py::object x = map[0]; // implicit conversion to py::object, calls PyObject_GetItem

The code is here.

Usefulness of const is limited considerably because how easy it is to drop it, even by accident.

I agree, but this applies to most of c++ since const can pretty much always be const_cast away.

[ghost]

@wjakob what is your opinion about this?

I will also add that apart from already mentioned PyCXX, the boost::python
library also tries to make const transitive. They even solve the last problem
with copy constructors to some degree. In boost::python copy constructor for
dict / list types creates a copy of underlying Python object, replicating
Python behaviour of dict(other), or list(other).

While I still think the benefit of transitive const is limited, I don't have
intention of pushing this change, if you disagree.

[wjakob]

Ultimately pybind11::object etc. are just like std::shared_ptr<...>, hence makingconst`` consistent with a wrapped pointer makes the most sense to me. Thank you for your contribution!