I’m looking for advice on how to implement returning of 3-element vector from a VTK method in the simplest and most Python-friendly way.
The vector value is not stored in a class member variable (it is computed dynamically). The value may be invalid and I would like to make this information conveniently available in Python (for example, by returning None instead of a valid vector).
These are the options that we have been using, but each has some limitations:
Good: we can easily return a valid vector or nullptr/None in both C++ and Python
Bad: we would need to have a temporary buffer to store the returned vector, but then who will delete that buffer?
Option B: vtkVector3d GetPoint(double x)
Good: vector is returned by value, no need for temporary buffer
Bad: we cannot indicate that the result is invalid
Adding a Valid flag to vtkVector3d could help, but it would be somewhat overkill for this special case, and the Python wrapper would need to convert an invalid vector to None
Using some special value, such as (NaN, NaN, NaN) could indicate invalid state without an additional flag, but it would not be very convenient to use
Option C: bool GetPoint(double point[3], double x) (returns true if the result is valid)
Good: works, does not require temporary buffer and can indicate invalid result
Bad: not intuitive or convenient for Python developers
Are there any better suggestions?
@dgobbi maybe the new VTK property wrappers could help in some way?
Option A could be supported with a hint to tell the wrappers to delete the return value, this would be nicely automated and invisible from Python but ugly from C++.
Option B is a no-go, the vtkVector types are widely used, and adding an “invalid” property is a big change in semantics.
Option C is my favorite since it doesn’t involve null-pointers or memory management, and once people see an example of how it’s used, it becomes reasonably intuitive.
Another option is to return a std::vector. For an invalid point, the vector could be empty.
As a pythoh user I would prefer and array with “nan” over maybe an array else None
It is not uncommon for invalid results to return an array with nan, for example np.sqrt(-1) returns np.float64(nan) but it will also output a runtime warning “:1: RuntimeWarning: invalid value encountered in sqrt
np.sqrt(-1)” that doesn’t block the program but give the user info that something happened
I quite like the std::vector idea. Python behavior would be almost exactly what we need (the only imperfection I see is that there would be no hints that the returned vector can only have 0 or 3 elements). In C++, it is OK, too (it is just a but heavier than a fixed array and always need to check the returned vector size).
Returning NaN-s cash work, too, but it is easier to forget about the value check and the check is a bit me work (need to check 3 values for NaN).
@ben.boeckel thanks for the idea, std::optional<vtkVector3d> is nice. Of all the options that have come up, I find this the most elegant. The Python wrapper does not recognize it yet.
@dgobbi what do you think, about adding Python support for std::optional<vtkVector3d>?
Do you think std::optional<vtkVector3d> could become a common pattern in VTK for cases when optional vector is returned by value, or VTK developers would prefer something like Option C?
I can not speak to the c++ side, but for the python side would std::optional resolve to a vtkVector3d instance or None in this case. I assume this is for pythonic property attributes that call a getter for data that would have been explicitly accessed with the obj.GetSomething previously.
The Python wrapper could implement any behavior. Returning None would probably make the most sense, because it would be consistent with the behavior when a vector is returned as double* (and the value is nullptr). Throwing some exception could be acceptable, too, but then maybe returning nullptr for a double* pointer should throw an exception, too.
An exception would be quite unexpected, I think. std::none would be None in Python and an “engaged” (that is the standardese for it) std::optional be a wrapped vtkVector3d object.
std::optional is an explicit acknowledgement that it can return “nothing”, so it is far less ambiguous than a T* return type (where only the docs can help whether nullptr is a sensible return value).
Yes, that sounds like a good way to handle std::optional<T>. The Python user would only ever see a T or None, because the wrappers would automatically do the has_value() check for the benefit of the user.
I’m in agreement with Ben regarding T*. Returned pointers are horridly ambiguous from C++, and can only be handled nicely in Python via an abundance of hints.
I would recommend against raising errors if the object has capacity to perform other valid computations whether or not this property is valid. For instance during a debug session in visual studio code depending on the users config the variable explorer may automatically call the getter method for the objects in scope. If the validity of the vector is not pertinent to what the user is doing and the object is still otherwise capable perform usable calculations that do not touch the vector it could lead to mis-direction when a user is iterating their code. Logging a warning may be useful as the user can control whether they see specific warnings.
Of course I would not consider adding C++ exceptions to VTK. It just could have been an option for Python, as you suggested above: sqrt(-1) returns nan in C++ but throws an exception in Python. But it seems we all agree on returning None would be more convenient.
This is indeed a side effect of the new VTK property style wrapping, but it should not cause major trouble.
We have been calling all Get... methods of VTK objects automatically in 3D Slicer’s Python console for years (for auto-completion) and the only thing we had to fix was suppressing error/warning messages. Occasionally we run into a crash during auto-complete due to calling a Get function in an unexpected state, but it is very rare (we come across one in every few years) and the issue is easy to find and fix.