Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to modulate the tube filter's radius in absolute units #3649

Merged
merged 6 commits into from
Nov 28, 2022

Conversation

jmargeta
Copy link
Contributor

Overview

This PR adds an argument to the tube filter so that the radius can be specified in absolute units for convenience.

spline.tube(scalars="radius", absolute=True)

Internally, it uses the SetVaryRadiusToVaryRadiusByAbsoluteScalar of vtkTubeFilter.

Details

  • support to modulate the tube's radius in absolute units, the default behavior remains unchanged
  • added an example to the create_spline section

Screenshot from 2022-11-27 01-16-29

 - Added absolute argument to turn this on, the detault behaviour
   modulating the radius relatively is left unchanged
 - Added an example to the spline section, however this
   works just as well with any line mesh
@github-actions github-actions bot added the documentation Anything related to the documentation/website label Nov 27, 2022
@jmargeta jmargeta changed the title Add support to modulate the tube filter in absolute units Add support to modulate the tube filter's radius in absolute units Nov 27, 2022
@codecov
Copy link

codecov bot commented Nov 27, 2022

Codecov Report

Merging #3649 (2f75cdb) into main (8c3f6d9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #3649   +/-   ##
=======================================
  Coverage   95.12%   95.12%           
=======================================
  Files          83       83           
  Lines       18556    18558    +2     
=======================================
+ Hits        17652    17654    +2     
  Misses        904      904           

Copy link
Member

@tkoyama010 tkoyama010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. Great contribution.

pyvista/core/filters/poly_data.py Outdated Show resolved Hide resolved
pyvista/core/filters/poly_data.py Outdated Show resolved Hide resolved
jmargeta and others added 2 commits November 27, 2022 03:12
Co-authored-by: Tetsuo Koyama <tkoyama010@gmail.com>
Co-authored-by: Tetsuo Koyama <tkoyama010@gmail.com>
tkoyama010
tkoyama010 previously approved these changes Nov 27, 2022
Copy link
Member

@tkoyama010 tkoyama010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jmargeta
Copy link
Contributor Author

Thanks a lot for the speedy review, @tkoyama010 !

tkoyama010
tkoyama010 previously approved these changes Nov 27, 2022
Copy link
Member

@adeak adeak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added things look great overall, just two (final) unrelated issues revealed by your PR (one only a nitpick) :)

tests/test_polydata.py Outdated Show resolved Hide resolved
pyvista/core/filters/poly_data.py Outdated Show resolved Hide resolved
Copy link
Member

@adeak adeak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks a lot @jmargeta!

@adeak adeak merged commit 936a02a into pyvista:main Nov 28, 2022
@jmargeta jmargeta deleted the enh/absolute-tube-radius branch November 28, 2022 22:08
@banesullivan banesullivan mentioned this pull request Feb 1, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Anything related to the documentation/website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants