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

Update and fix connectivity docs #4872

Merged
merged 8 commits into from Sep 15, 2023
Merged

Conversation

user27182
Copy link
Contributor

@user27182 user27182 commented Sep 11, 2023

Overview

Follow up from #4824. Improve docs and include minor bug fixes.

Details

  • Fix compute-volume example to use the correct connectivity scalar_range (percentages taken from threshold_percent were previously incorrectly used)
  • Fix list formatting for versionadded directive and variable_input param (lists were previously formatted as a paragraph)
  • Remove single float use case for scalar_range (must be a sequence with two values) and add relevant test cases
  • Update See Also section with relevant methods (methods are now also hyperlinked)
  • Move long examples to examples/01-filter/connectivity.py
  • Add new short docstring examples
  • Other minor documentation fixes to improve readability / clarity
  • Fix bug with 'specified' mode where specifying an unused region id would raise an unexpected KeyError. The filter now correctly returns an empty mesh.
  • Fix bug with PolyData.clean where it would would raise an error when cleaning a mesh with points but no cells

@github-actions github-actions bot added the documentation Anything related to the documentation/website label Sep 11, 2023
@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Merging #4872 (fea9233) into main (279c9ff) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #4872   +/-   ##
=======================================
  Coverage   95.96%   95.96%           
=======================================
  Files         130      130           
  Lines       21465    21476   +11     
=======================================
+ Hits        20598    20609   +11     
  Misses        867      867           

@tkoyama010 tkoyama010 changed the title Update/fix connectivity docs Update and fix connectivity docs Sep 11, 2023
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.

Thank you for suggesting the correction, I do have one suggestion. The docstring Example section of connectivity seems a bit long in my mind. Could you move that content to examples/01-filter/connectivity.py? For us, the docstring is an introduction to using the method by default in a simple way, and we would like to see a separate example page for more detailed usage.

@user27182
Copy link
Contributor Author

Could you move that content to examples/01-filter/connectivity.py?

Ok. I agree this is a better place for these. The examples have been moved with some edits / changes, and new, simpler, docstring examples added. See the updated PR Overview for details.

examples/01-filter/connectivity.py Outdated Show resolved Hide resolved
examples/01-filter/connectivity.py Outdated Show resolved Hide resolved
tests/core/test_polydata.py Outdated Show resolved Hide resolved
tests/core/test_polydata.py Outdated Show resolved Hide resolved
pyvista/core/filters/data_set.py Outdated Show resolved Hide resolved
pyvista/core/filters/poly_data.py Outdated Show resolved Hide resolved
tkoyama010
tkoyama010 previously approved these changes Sep 14, 2023
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. Thanks!

@tkoyama010 tkoyama010 merged commit 9c53f64 into pyvista:main Sep 15, 2023
23 checks passed
@user27182 user27182 deleted the doc/connectivity branch September 15, 2023 16:36
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