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

REFACTOR: Extend consistency on plot methods #4723

Merged
merged 8 commits into from
May 27, 2024

Conversation

SMoraisAnsys
Copy link
Collaborator

@SMoraisAnsys SMoraisAnsys commented May 24, 2024

Following #4626 and #4708, theses changes aim at making our code base consistent.

The changes consist in returning matplotlib figure objects on methods that were returning a list of value.
In the case of polar_plot_3d_pyvista, a pyvista plotter object is returned when, before, a boolean was returned if one wanted to show the figure or save it in a file.

This changes should be performed before merging #4702

Following #4626 and #4708, theses changes aim at making our
code base consistent. The changes consist in returning Matplotlib
figure objects on methods that were returning a list of value.
In the case of polar_plot_3d_pyvista, a Pyvista plotter object
is returned when, before, a boolean was returned if one wanted to
show the figure or save it in a file.
@ansys-reviewer-bot
Copy link
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

Devin-Crawford
Devin-Crawford previously approved these changes May 24, 2024
Copy link
Contributor

@Devin-Crawford Devin-Crawford left a comment

Choose a reason for hiding this comment

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

See minor suggestions, otherwise looks good.

@codecov-commenter
Copy link

codecov-commenter commented May 24, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 80.42%. Comparing base (4c4932b) to head (9d61952).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4723   +/-   ##
=======================================
  Coverage   80.42%   80.42%           
=======================================
  Files         121      121           
  Lines       55116    55071   -45     
=======================================
- Hits        44328    44293   -35     
+ Misses      10788    10778   -10     

@Devin-Crawford
Copy link
Contributor

All good.

Copy link
Contributor

@Devin-Crawford Devin-Crawford left a comment

Choose a reason for hiding this comment

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

LGTM.

@Devin-Crawford Devin-Crawford self-assigned this May 27, 2024
@Devin-Crawford
Copy link
Contributor

Inadvertently closed. LGTM.

@Devin-Crawford Devin-Crawford marked this pull request as ready for review May 27, 2024 06:05
@Devin-Crawford Devin-Crawford marked this pull request as draft May 27, 2024 06:06
@SMoraisAnsys SMoraisAnsys marked this pull request as ready for review May 27, 2024 06:53
Copy link
Member

@Samuelopez-ansys Samuelopez-ansys left a comment

Choose a reason for hiding this comment

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

LGTM

@SMoraisAnsys SMoraisAnsys merged commit a67460f into main May 27, 2024
34 checks passed
@SMoraisAnsys SMoraisAnsys deleted the refactor/extend-consistency-on-plot-methods branch May 27, 2024 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants