ENH: Making header_style a property of ExcelFormatter #22758 #22759
Conversation
|
Hello @dannyhyunkim! Thanks for submitting the PR.
|
|
Nice job! Can you add a whatsnew note for 0.25? |
Codecov Report
@@ Coverage Diff @@
## master #22759 +/- ##
==========================================
+ Coverage 92.16% 92.17% +0.01%
==========================================
Files 169 169
Lines 50769 50770 +1
==========================================
+ Hits 46791 46799 +8
+ Misses 3978 3971 -7
Continue to review full report at Codecov.
|
|
@WillAyd , wasn't entirely sure if this was what you were after, but I had a crack! |
|
This is totally my mistake but I meant v0.24 (a whatsnew should already exist). You did a great job in adding one but unfortunately that’s not what we want here. Sorry! |
|
No worries at all. I should have clarified earlier. |
doc/source/whatsnew/v0.24.0.txt
Outdated
| @@ -185,6 +185,7 @@ Other Enhancements | |||
| - :class:`Resampler` now is iterable like :class:`GroupBy` (:issue:`15314`). | |||
| - :meth:`Series.resample` and :meth:`DataFrame.resample` have gained the :meth:`Resampler.quantile` (:issue:`15023`). | |||
| - :meth:`Index.to_frame` now supports overriding column name(s) (:issue:`22580`). | |||
| - :class:`ExcelFormatter` now has a `header_style` property. Was previously in global scope (:issue:`22758`). | |||
There was a problem hiding this comment.
Might not actually need this since the ExcelFormatter isn't part of the exposed API
There was a problem hiding this comment.
Should it be? If we want to support use-cases like #22758 then it should be. OTOH, I don't think we consider things like HTMLFormatter to be public.
There was a problem hiding this comment.
Yea I can see it both ways but was leaning towards no for now just because I don't want this change to make assumptions about our overall strategy with the formatters.
If we want to I'd suggest a separate change that focuses on exposing and documenting the formatters rather than doing it here
There was a problem hiding this comment.
That sounds right to me. (and there's nothing stopping people from using it anyway).
There was a problem hiding this comment.
Opened #22773 to further that discussion; @dannyhyunkim if you can revert the change to whatsnew would be great as this in its current state is more of an internal refactor. Sorry for the back and forth from me on that!
|
@WillAyd : Thanks for the ping. Will take a look later today. |
|
Thanks @dannyhyunkim - great first contribution! |
git diff upstream/master -u -- "*.py" | flake8 --diff