Skip to content

test: add unit tests for rgb/hex color conversion helpers#5659

Open
eeshsaxena wants to merge 1 commit into
plotly:mainfrom
eeshsaxena:test/colors-helpers
Open

test: add unit tests for rgb/hex color conversion helpers#5659
eeshsaxena wants to merge 1 commit into
plotly:mainfrom
eeshsaxena:test/colors-helpers

Conversation

@eeshsaxena

Copy link
Copy Markdown

Description

The rgb/hex color-conversion helpers in _plotly_utils/colors were not directly covered. This adds tests/test_core/test_colors/test_color_conversions.py covering:

  • hex_to_rgb for basic hex values
  • label_rgb formatting a tuple and unlabel_rgb parsing it back (and their round-trip)
  • find_intermediate_color at the midpoint, at both endpoints, and with colortype="rgb"

No source changes.

Verification

pytest tests/test_core/test_colors/test_color_conversions.py - 7 passed. black clean.

Comment on lines +46 to +49


if __name__ == "__main__":
raise SystemExit(pytest.main([__file__, "-v"]))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't use this pattern in our Pytest tests

Suggested change
if __name__ == "__main__":
raise SystemExit(pytest.main([__file__, "-v"]))

Comment on lines +1 to +2
import pytest

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
import pytest

@emilykl emilykl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@eeshsaxena Thanks for the PR!

By convention, all tests for _plotly_utils are located in tests/test_plotly_utils/.

Can you move this test file to tests/test_plotly_utils/colors/test_color_conversions.py?

You may need to add a tests/test_plotly_utils/colors/__init__.py file as well.

Otherwise, looks good and we're happy to merge following that change.

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.

2 participants