Ignore the window when picking points

[p-j-smith]

Overview

Fixes #1907
This PR is based on @banesullivan's solution to the discussion of this topic in #1894

Allows points in the 3D window to be ignored after enabling point picking.

Details

• A window_pickable keyword has been added to pyvista.plotting.picking.PickingHelper.enable_point_picking(). If False, all points in the 3D window will be ignored by the _end_pick_event callback. If True (the default), the points in the window are pickable (the current behaviour).

• A basic example of point picking and ignoring the 3D window has been added to examples/02-plot/point-picking. I can either flesh out the example if it's desirable, or just remove the file if you prefer

[codecov]

Codecov Report

Merging #1908 (a107395) into main (06b1ac0) will decrease coverage by 0.02%.
The diff coverage is 22.72%.

@@            Coverage Diff             @@
##             main    #1908      +/-   ##
==========================================
- Coverage   30.86%   30.84%   -0.03%     
==========================================
  Files          74       74              
  Lines       15371    15394      +23     
==========================================
+ Hits         4744     4748       +4     
- Misses      10627    10646      +19     

[banesullivan]

I'm not a huge fan of this... I'd like to avoid having any examples directly utilize the underlying VTK methods if possible (albeit, one of PyVista's selling points is direct access back to VTK when you want to do something more sophisticated).

Either way, this doesn't feel all that Pythonic/PyVista-ish to me

[p-j-smith]

Yeah it's not particularly Pythonic. How about creating a method p.set_pickable_actors that takes a list of vtk actors to make pickable? Also, should the fine grained control of pickable actors be addressed by a separate issue/PR?

[p-j-smith]

Hi @banesullivan , I've added a new property pickable_actors to the BasePlotter. It can be used to set and retrieve the pickable actors of a given plotter. This means the 3d window can be made pickable independently of which actors are pickable, e.g. if a limit_to_actors argument were added to enable_point_picking, then it would not be possible to make the 3d window pickable and all actors unpickable. I've also added tests for the new property. Do you think this is a reasonable approach to take? Also, I wasn't sure where to add things, so let me know if you want me to move stuff around.

[akaszynski]

I'd like to see full coverage for this new feature. Can we get additional unit tests for this?

[p-j-smith]

I'd like to see full coverage for this new feature. Can we get additional unit tests for this?

I think something strange is going on with codecov. It reports that the total coverage of pyvista is 30.86%, but when I run the tests with coverage locally (pytest -vv --cov-report=html --cov-config=.coveragerc --cov pyvista) I get the following:

     	Statements  Missing	Excluded Coverage
Total	15393	    1375	975	 91%

Also, codecov shows that none of enable_point_picking is covered (lines 288-319). However, even before this PR there is a test (tests/tests_pickling.py:test_point_picking) that covers this function.

The reports I've generated locally show that all lines I've modified or added are covered by unit tests - see lines 288-319 of d_ec8005e13284025b_picking_py.html and lines 1274-1308 of d_ec8005e13284025b_plotting_py.html, both of which are in coverage.zip.

Do you have any ideas why codevoc might be under-reporting by so much (91% vs 30.84%)?

Edit

Fix the formatting that I messed up when adding coverage.zip

[p-j-smith]

It looks like codecov has been wildly inaccurate for the past few weeks. This is where total coverage dropped by over 65%: https://codecov.io/gh/pyvista/pyvista/commit/b76c8ce08424d16c893248537f042fe1d60c98d6 But it's not obvious why it dropped here

[akaszynski]

Thanks for looking into this. It's difficult to say if this is a pyvista or upstream issue.

I'm checking out this PR locally and will test and review it.

[akaszynski]

Made some final changes to the docstrings as the setter docstring isn't rendered in our autodocumentation. Also added some examples and links.

Overall this is good work. Thanks for your contribution!

[p-j-smith]

Thank you both for the reviews!