Dark background #40836
Dark background #40836
Conversation
…with 'dark_background' theme pandas-dev#40769
|
Hello @yeshsurya! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-05-06 07:17:23 UTC |
There was a problem hiding this comment.
Thanks for the pr, @yeshsurya! Could you please add a whatsnew note describing what is fixed by this patch? Looks like there are some test failures (https://github.com/pandas-dev/pandas/pull/40836/checks?check_run_id=2298265089)
Also looks like some style checks are failing, this portion of the contributing guide should be helpful there (https://pandas.pydata.org/docs/development/contributing.html#pre-commit).
Could you also post a before/after of what this change does with respect to the original issue?
| dark_result = df.boxplot(return_type="dict") | ||
| for k, v in dark_expected.items(): | ||
| assert dark_result[k][0].get_color() == v | ||
| plt.style.use('defult') |
There was a problem hiding this comment.
Should this be
| plt.style.use('defult') | |
| plt.style.use('default') |
?
| {"boxes": "#1f77b4","whiskers": "#1f77b4","medians":"#2ca02c","caps":"#1f77b4"}) | ||
| ], | ||
| ) | ||
| def test_colors_in_theme(self, dark_expected,default_expected): |
There was a problem hiding this comment.
this parametrising has no purpose since there is only one item in the list. You might as well define these variables in the function.
what is better is to restructure it so you only test one thing at a time:
e.g:
@pytest.mark.parametrize(
"scheme, expected",
[
('dark_background', {"boxes": "#8dd3c7","whiskers": "#8dd3c7","medians":"#bfbbd9","caps":"#8dd3c7"}),
('default', {"boxes": "#1f77b4","whiskers": "#1f77b4","medians":"#2ca02c","caps":"#1f77b4"})
],
)
then the code for your test can be half as long.
yes , it shouldn't be. I was invoing df.boxplot() in test but I should be testing df.plot.box(). I've changed this. |
|
looks fine if you can add a whatsnew note as @charlesdong1991 suggests and ping on green (bug fix section of 1.3 in visualization) |
Added whatsnew note |
did you push ? it doesn't show up |
Oops ! Missed it. Pushed it now . |
* times in ewm groupby: sort times in according to grouping; add missing support for times in numba implementation; fix bug in cython implementation * add GH issue id to tests * fix typing validation error * PR comments * trying to fix int64 to int32 casting TypeError * PR comments * PR comments * PR comments
…das-dev#41004) * fix: function _take_with_is_copy was defined final but overriden * fix: add return type
* TYP: get_indexer * update per discussion in pandas-dev#40612 * one more overload * pre-commit fixup
|
Another cleaner pull request raised for this : #41349 |
|
emm, okay, @yeshsurya , then I will close this PR to make way for the other one |
Before Fix :
After Fix :
Whats New : Box Plot's Caps will have same color as boxes unless color is explicitly specified by user arguments. Hence theme changes will not adversly effect caps color.