139 better color class #140
Conversation
bbean23
commented
Jul 30, 2024
bbean23
commented
- add more conversion methods (to/from hex, to/from HSV)
- expand on matplotlib color palette definition
- add build_colormap() method
- add unit tests
… palette definition, add build_colormap() method
| self.assertAlmostEqual(color_map(0.5)[0], 0.5, places=2) | ||
| self.assertAlmostEqual(color_map(0.5)[1], 0.0, places=2) | ||
| self.assertAlmostEqual(color_map(0.5)[2], 0.5, places=2) | ||
|
|
There was a problem hiding this comment.
For clarification, this is testing the middle of the colormap of red and blue, so the color purple?
There was a problem hiding this comment.
Yep, that's correct. Do you think it needs a comment clarifying that?
| r2, g2, b2 = mpl_colors[i] | ||
| self.assertAlmostEqual(r1, r2) | ||
| self.assertAlmostEqual(g1, g2) | ||
| self.assertAlmostEqual(b1, b2) |
There was a problem hiding this comment.
Is this comparing the _plotColors class with the matplotlib tab10 colors?
Why would plot_colors_names also not be tested in the unit tests? Is this since throughout this file it calls the colors by name? Such as cl.blue
There was a problem hiding this comment.
The purpose of this test isn't to verify that the color names match those from matplotlib, but rather to verify that the color values continue to match those in matplotlib into the future. I added the following comment to this test:
The matplotlib tab10 colors are encoded in color.py. This test is here
to catch any change in the matplotlib colors, if there ever is any.
Hopefully this answers your question?
There was a problem hiding this comment.
Is it unnecessary to test for invalid Hex strings and HSV values?
There was a problem hiding this comment.
Two answers:
- Hopefully not. Hopefully those invalid values will be caught by the underlying matplotlib library calls.
- I could add those tests, but that's starting to lean towards the excessive side of the "get it done" vs "test everything" balancing act. The interaction then between the author and the reviewer then determines if it is necessary.
