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

Make colors iterable and indexable #2415

Merged
merged 6 commits into from
Apr 5, 2022
Merged

Conversation

dcbr
Copy link
Contributor

@dcbr dcbr commented Apr 2, 2022

Overview

Support for pyvista.Color()[i] and for c in pyvista.Color() to make it backwards compatible with the old 'float tuple' representation.
Fixes #2409.

Details

The float_rgba representation is used for indexing and iteration.

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

codecov bot commented Apr 2, 2022

Codecov Report

Merging #2415 (f4fee55) into main (542b37f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2415   +/-   ##
=======================================
  Coverage   93.63%   93.64%           
=======================================
  Files          74       74           
  Lines       15984    15994   +10     
=======================================
+ Hits        14967    14977   +10     
  Misses       1017     1017           

pyvista/plotting/colors.py Outdated Show resolved Hide resolved
pyvista/plotting/colors.py Show resolved Hide resolved
pyvista/plotting/colors.py Outdated Show resolved Hide resolved
@adeak
Copy link
Member

adeak commented Apr 2, 2022

Ah, and codecov has a point: we also need tests.

@akaszynski
Copy link
Member

Ah, and codecov has a point: we also need tests.

Amazing when your CI helps you!

pyvista/plotting/colors.py Outdated Show resolved Hide resolved
pyvista/plotting/colors.py Outdated Show resolved Hide resolved
@adeak
Copy link
Member

adeak commented Apr 3, 2022

I've pushed some changes (it was easier than adding them as suggestions). Please review and check if you agree. I think coverage should still be OK but I haven't checked, will have to look at CI.

pyvista/plotting/colors.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.

Approving with a single remark. Not sure about downstream code.

pyvista/plotting/colors.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.

LGTM, thanks!

@akaszynski akaszynski merged commit 0856b08 into pyvista:main Apr 5, 2022
@dcbr dcbr deleted the fix/colors_iter branch April 5, 2022 18:12
akaszynski pushed a commit that referenced this pull request Apr 18, 2022
* Quick implementation

* Apply code review suggestions, support string indexing, add tests

* Apply suggestions from code review

* Update channel names and __getitem__

* Add tests

* Support negative indices and slices

Co-authored-by: Andras Deak <deak.andris@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Uh-oh! Something isn't working as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pv.background_color is not iterable in v0.34.0
3 participants