-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: Limited available colors #38247
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
Changes from 2 commits
38f515d
675415b
d6e919c
dde07c0
0f74d17
2430400
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
|
||
ExcelFormatter is tested implicitly in pandas/tests/io/excel | ||
""" | ||
import string | ||
|
||
import pytest | ||
|
||
|
@@ -313,3 +314,9 @@ def test_css_to_excel_bad_colors(input_color): | |
with tm.assert_produces_warning(CSSWarning): | ||
convert = CSSToExcelConverter() | ||
assert expected == convert(css) | ||
|
||
|
||
def tests_css_named_colors_valid(): | ||
upper_hexs = set(map(str.upper, string.hexdigits)) | ||
for color in CSSToExcelConverter.NAMED_COLORS.values(): | ||
assert len(color) == 6 and all(c in upper_hexs for c in color) | ||
Comment on lines
+321
to
+324
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you consider adding a test that all colors from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I wondered a bit about that, but didn't make my mind. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we just import these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possible of course, but is it worth introducing a dependency on mpl for excel manipulation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds reasonable for me to have a list of colors, rather than depend on matplotlib.
Could think of placing the list into a separate module, so that
pandas/io/formats/excel.py
is not cluttered?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used matplotlib nomenclature (_color_data.py), maybe something else will be more adequate