vtkSphere : passing a null parameter in ComputeBoundingSphere from Python

[chir-set (SET)]

Hello,

I’m calling vtk.vtkSphere.ComputeBoundingSphere() from Python and stumbling on passing the hints[2] parameter. I don’t want to determine the 2 values of this parameter since vtkSphere does it on its own if a nullptr is passed. However, passing None from python fails:

TypeError: ComputeBoundingSphere argument 4: expected a sequence of 2 values, got NoneType

In this particular case, passing an arbitrary valid range will be OK since all points will be enclosed in the resulting sphere, and the result is tagged as approximate (comments in the cxx file).

I wish to know if there’s a way to pass a real nullptr from Python in general. Searching the web did not help.

Thank you.

[rexthor]

Hi and welcome to the group.
I wish there were more documentation in the documentation for that method. I got this to run … but I’m not entirely sure I know what I asked VTK to do.

from vtkmodules.vtkCommonDataModel import vtkSphere
import numpy as np

s = vtkSphere()
points = np.random.rand(5, 3).ravel() # 5 3D points, selected at random
result = [0., 0., 0., 0.]
s.ComputeBoundingSphere(points, 5, result, [0, 1])
print(result)

I’m guessing that the result is (center_x, center_y, center_z, radius).

[chir-set (SET)]

@rexthor

[0, 1]

You mean pass anything when we don’t know because we cannot say we don’t know (passing None) in Python?

Indeed, a little more precision in the documentation would have saved much time. I was about to open a merge request to change ‘hints[2]’ to ‘hints[2] = nullptr’, I think now it’s superfluous.

Thank you.

@rexthor

I’m guessing that the result is (center_x, center_y, center_z, radius).

Yes, that’s documented.

[rexthor]

Oh - I think in this case, I passed in [0, 1] because I didn’t want to bother figuring out where the farthest points might have been. In someone else’s case, where there might have been thousands or millions of points - that might have made the difference between seconds and days.

Passing references from Python to C++ is … well … not fun. I’ve tried it before in other toolkits, but VTK just makes it so easy. I think that whoever wrote this interface wanted a Python object that looks like a list … if you pass it a None, it isn’t list-like. You might try passing [None, None] and it might work as well.

[lassoan (Andras Lasso)]

If you pass “nullptr” then the function will compute the point index hints in a smart way that reduces the computation time and overestimation of the bounding sphere. If you just use the first two point indices as hints (passing [0, 1] vector as the last parameter) as it was suggested above then the method may run much slower and the radius may be way off.

@dgobbi Is there way to pass a “nullptr” from Python as hints parameter to vtkSphere::vtkSphereComputeBoundingSphere(T* pts, vtkIdType numPts, T sphere[4], vtkIdType hints[2])?

[dgobbi (David Gobbi)]

@lassoan

Is there way to pass a “nullptr” from Python as hints parameter

No. Since it’s declared as “vtkIdType hints[2]”, the Python wrappers assume that the function does not expect it to be null. It would be unsafe to assume otherwise, unless the wrappers received some hint that the function checked for null.

[dgobbi (David Gobbi)]

Though this won’t help people using the current version of VTK, vtkSphere should add an overload that defaults hints to nullptr:

template <class T>
void ComputeBoundingSphere(T* pts, vtkIdType numPts, double sphere[4])
{
  return ComputeBoundingSphere(pts, numPts, sphere, nullptr);
}

Edit: I see this has already been suggested above.

[lassoan (Andras Lasso)]

@dgobbi

Since it’s declared as “vtkIdType hints[2]”, the Python wrappers assume that the function does not expect it to be null. It would be unsafe to assume otherwise,

Thank you, this makes sense.

@dgobbi

vtkSphere should add an overload that defaults hints to nullptr

@chir-set could you submit a merge request with this change?

[chir-set (SET)]

@lassoan

could you submit a merge request with this change?

Ok, I can do that.

I request one precision.

a. Should it be an additional function to the vtkSphere class as written by @dgobbi ?

b. Or should it a modification of existing function signatures, replacing ‘hints[2]’ by ‘hints[2] = nullptr’?

I understand it should be #a .

[dgobbi (David Gobbi)]

Yes, overloads are preferred over default arguments.

https://docs.vtk.org/en/latest/developers_guide/coding_conventions.html

35. Prefer overloading functions to default arguments.
    Rationale: Default function arguments in C++ are a tempting way to add an argument to a function while maintaining easy backwards compatibility. However, if you later want to add another argument to the list in a way that preserves backwards compatibility, it, too, must be a default argument. To supply the second of these arguments in a call forces you to also supply the first argument, even if it is the default value. As a result, this is not a clean way to add a argument to a function. Insetad, function overloading should be preferred.

[lassoan (Andras Lasso)]

I’m not sure if there is a clean way to set default for a double[2] argument, so I fully agree that overload is a better approach in this case. In general, I’m not sure that overloads is always preferable (and if they are preferable then not because they are better for managing backward compatibility), but that’s a separate discussion topic in itself - so I’ve added a new topic for this:

Default argument value vs. overload

Recently the following was added to the VTK coding conventions document: Prefer overloading functions to default arguments. Rationale: Default function arguments in C++ are a tempting way to add an argument to a function while maintaining easy backwards compatibility. However, if you later want to add another argument to the list in a way that preserves backwards compatibility, it, too, must be a default argument. To supply the second of these arguments in a call forces you to also supply th…