Create PyvistaFutureWarning, use it to warn about extrude capping changes

[adeak]

Overview

As per the discussion in #2339 we should change back the value of the capping kwarg to True to match VTK. As a first approach I kept the original kwarg but added a future warning (with a new non-silenced warning class), because code written in the last 6 months might rely on the new False default.

We could also argue that the change in default behaviour was a bug and we could also fix it (i.e. just revert the default to True). Let me know if this would be preferred.

I haven't built the docs locally, I'll check the CI artifact.

[codecov]

Codecov Report

Merging #2364 (67d0c18) into main (611e93e) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2364      +/-   ##
==========================================
- Coverage   93.53%   93.51%   -0.02%     
==========================================
  Files          74       74              
  Lines       15723    15747      +24     
==========================================
+ Hits        14706    14726      +20     
- Misses       1017     1021       +4     

[tkoyama010]

IMHO: It will be good to add tests of default arg using GetCapping() in vtk object.

[adeak]

IMHO: It will be good to add tests of default arg using GetCapping() in vtk object.

Thanks @tkoyama010. What would we test with that? That the default is what we think it is (i.e. True)?

[tkoyama010]

That the default is what we think it is (i.e. True)?

@adeak Yes, it is.

[adeak]

I see, thanks. I'm not sure we want to do that: probably everywhere in PyVista we have assumptions about the underlying VTK filters. Even if we expect that VTK changes this default in the future, I would argue that PyVista users should not be affected by this. We can hide VTK's changes and stick to the new True default once and for all.

We should definitely check though that the current VTK default is True. But I did that, and you did too in #2339 :) What do you think?

[akaszynski]

Agree with a future deprication, probably one or two minor releases.

I would like to see coverage of the warning, but I’m not going to block this PR.

[adeak]

I would like to see coverage of the warning, but I’m not going to block this PR.

Done.

[tkoyama010]

LGTM. Thanks!

[adeak]

Sorry, I've disabled auto-merge because I haven't looked at the built docs yet. I'll post a comment when I'm done with that.

[adeak]

I missed two places where warnings would've been raised by doctest examples, these are now fixed. This is ready for auto-merge, thank you.

[akaszynski]

Thanks for your work @adeak.