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.

Sign up for GitHub

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

Jump to bottom

Use Set method insted of On and Off method #3001

Merged
merged 6 commits into from Jul 16, 2022
Merged

Use Set method insted of On and Off method #3001

merged 6 commits into from Jul 16, 2022

Conversation

RobPasMue
Copy link
Contributor

Overview

Resolves #2769

Details

Only conditional statements in which On() and Off() statements were used have been refactored.

@github-actions github-actions bot added the bug Uh-oh! Something isn't working as expected. label Jul 16, 2022
@codecov
Copy link

codecov bot commented Jul 16, 2022 •
edited

Codecov Report

Merging #3001 (022fa1f) into main (3eba9a6) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #3001      +/-   ##
==========================================
- Coverage   94.31%   94.31%   -0.01%     
==========================================
  Files          76       76              
  Lines       16566    16557       -9     
==========================================
- Hits        15625    15616       -9     
  Misses        941      941

tkoyama010
tkoyama010 requested changes Jul 16, 2022
View reviewed changes
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.

Good catch. I have some suggestions. It is better to have an explicit code in "else" case.

pyvista/plotting/scalar_bars.py Show resolved Hide resolved
pyvista/plotting/scalar_bars.py Outdated Show resolved Hide resolved
pyvista/plotting/renderer.py Outdated Show resolved Hide resolved
pyvista/plotting/renderer.py Outdated Show resolved Hide resolved
pyvista/core/filters/uniform_grid.py Show resolved Hide resolved
pyvista/core/filters/uniform_grid.py Show resolved Hide resolved
RobPasMue and others added 2 commits July 16, 2022 19:11
tkoyama010
tkoyama010 previously approved these changes Jul 16, 2022
View reviewed changes
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

@RobPasMue
Copy link
Contributor Author

Batch suggestions applied are causing test failures. Looking into issue

@RobPasMue
Copy link
Contributor Author

@tkoyama010 problem solved. Ready to merge.

akaszynski
akaszynski approved these changes Jul 16, 2022
View reviewed changes
Copy link
Member

@akaszynski akaszynski 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 5fc6d19 into pyvista:main Jul 16, 2022
@tkoyama010 tkoyama010 added the scipy2022 Sprint in scipy2022. label Jul 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akaszynski akaszynski akaszynski approved these changes

@tkoyama010 tkoyama010 Awaiting requested review from tkoyama010

Assignees
No one assigned
Labels
bug Uh-oh! Something isn't working as expected. scipy2022 Sprint in scipy2022.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use Set method insted of On and Off method
3 participants
@RobPasMue @tkoyama010 @akaszynski