Support more styles for xlsxwriter #16149

jnothman · 2017-04-26T16:45:44Z

I was surprised to find that despite the interchangeable representation of Excel styles, xlsxwriter did not have good style support.

I've not added direct tests for this functionality, but test some of it through test_styler_to_excel.

~~closes #xxxx~~
tests added / passed
passes git diff upstream/master --name-only -- '*.py' | flake8 --diff
whatsnew entry

jnothman · 2017-04-27T00:14:35Z

There is a consistent test failure, but not one I've managed to replicate locally.

codecov · 2017-04-27T01:03:45Z

Codecov Report

Merging #16149 into master will decrease coverage by <.01%.
The diff coverage is 89.65%.

@@            Coverage Diff             @@
##           master   #16149      +/-   ##
==========================================
- Coverage   90.83%   90.83%   -0.01%     
==========================================
  Files         159      159              
  Lines       50796    50809      +13     
==========================================
+ Hits        46143    46153      +10     
- Misses       4653     4656       +3

Flag	Coverage Δ
#multiple	`88.61% <89.65%> (-0.01%)`	⬇️
#single	`40.3% <3.44%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/excel.py	`80.55% <89.65%> (-0.07%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b80ed3...c935f5d. Read the comment docs.

codecov · 2017-04-27T01:03:47Z

Codecov Report

Merging #16149 into master will decrease coverage by 0.02%.
The diff coverage is 92.1%.

@@            Coverage Diff             @@
##           master   #16149      +/-   ##
==========================================
- Coverage   91.24%   91.21%   -0.03%     
==========================================
  Files         163      163              
  Lines       50091    50106      +15     
==========================================
+ Hits        45704    45706       +2     
- Misses       4387     4400      +13

Flag	Coverage Δ
#multiple	`89.02% <92.1%> (-0.01%)`	⬇️
#single	`40.23% <10.52%> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/excel.py	`80.39% <92.1%> (-0.01%)`	⬇️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.75% <0%> (-0.1%)`	⬇️
pandas/core/indexes/datetimes.py	`95.41% <0%> (-0.1%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5959ee3...80ed56a. Read the comment docs.

jnothman · 2017-04-27T06:09:34Z

Finally passing tests after identifying a recently fixed openpyxl bug that got in the way

jreback · 2017-04-27T10:25:31Z

pandas/io/excel.py


        if num_format_str is not None:
-            xl_format.set_num_format(num_format_str)
+            props['num_format'] = num_format_str



can we move more of this logic out of here an into the formats dir somewhere?

Do you mean explicitly moving the number format logic out? Yes, perhaps that's a worthwhile refactoring. I think we should also be calculating the number format from the display.precision config. For which reason, I believe all of those changes belong in a different PR.

Or are you talking about moving this mapping logic out? Well currently we assume nested style dicts as an interchange format, which are well-suited to openpyxl but need conversion for all writers. The stuff in formats/ should remain relatively writer-agnostic.

ideally all of this logic would just be a single call here and the logic elsewhere.

I don't think I've grokked your vision, given that this is writer specific. Do you mean that there should be more refactoring across writers? Except for this number formatting, it's already quite factored, as they each have different syntaxes for creating and formatting cells.

yes i think think excel should be refactored into a subdir of writer code and style things should live there

maybe make an issue about this
it's a bit of work to split it then adding things like style should be easy

jnothman · 2017-04-29T11:23:50Z

I'm happy to make an issue aiming to refactor excel writing code. But how do you feel about this PR?

jreback · 2017-06-10T19:03:41Z

can you rebase.

jnothman · 2017-06-14T13:05:58Z

I've moved the what's new to 0.20.

jreback · 2017-06-28T05:25:06Z

pandas/io/excel.py

+            style_dict = style_dict.copy()
+            style_dict['border'] = style_dict.pop('borders')
+
+        for src, dst in self.STYLE_MAPPING:


so this only is triggered if there is styling (IOW this won't cause a perf issue for 'regular' excel)?

A few lines above we return if style_dict is None; a few lines above that we return if num_format_str is None and style_dict is None. I think that is sufficient.

Btw, I think even the default to_excel has some styling of headers, so this function will always be called, but will be returned early where possible.

There are ways to make this faster, though:

store STYLE_MAPPING as a trie and descend recursively only where a prefix is matched.

flatten style_dict and store STYLE_MAPPING as a dict so that their keys match. But to be deterministic in case of multiple competing styles, STYLE_MAPPING would need to store the matched index, and the results would need to be sorted.

I'm pushing a faster variant.

jreback · 2017-06-28T05:28:16Z

pandas/io/excel.py

@@ -1609,6 +1609,68 @@ def write_cells(self, cells, sheet_name=None, startrow=0, startcol=0,
                          startcol + cell.col,
                          val, style)

+    # Map from openpyxl-oriented styles to flatter xlsxwriter representation


I think the code would be simpler to make this style formatting into a separate class (rather than have it live in functions sitting in the main excel code). can you refactor to make this cleaner.

jreback · 2017-08-17T10:35:13Z

this looks reasonable, can you rebase

jnothman · 2017-08-17T10:44:52Z

Yes, sorry I've not managed to do the refactoring you'd like to see. I've been unsure what you would like, and have had my attentions elsewhere.

…er-styles

jnothman

AppVeyor failure looks like someone else's problem.

jnothman · 2017-08-17T11:11:25Z

pandas/io/excel.py

+            style_dict = style_dict.copy()
+            style_dict['border'] = style_dict.pop('borders')
+
+        for src, dst in self.STYLE_MAPPING:


A few lines above we return if style_dict is None; a few lines above that we return if num_format_str is None and style_dict is None. I think that is sufficient.

Btw, I think even the default to_excel has some styling of headers, so this function will always be called, but will be returned early where possible.

There are ways to make this faster, though:

store STYLE_MAPPING as a trie and descend recursively only where a prefix is matched.

flatten style_dict and store STYLE_MAPPING as a dict so that their keys match. But to be deterministic in case of multiple competing styles, STYLE_MAPPING would need to store the matched index, and the results would need to be sorted.

jnothman · 2017-08-17T13:42:55Z

pandas/io/excel.py

+            style_dict = style_dict.copy()
+            style_dict['border'] = style_dict.pop('borders')
+
+        for src, dst in self.STYLE_MAPPING:


I'm pushing a faster variant.

… in use

jreback · 2017-08-18T00:54:19Z

cc @chris-b1 @TomAugspurger

…into xlsxwriter-styles

jnothman · 2017-10-16T00:26:21Z

Merge into 0.21 and avoid future what's new conflicts?

jreback · 2017-10-16T00:31:13Z

you need to rebase and some comments to respond

jnothman · 2017-10-16T01:08:26Z

Well, your comments suggested a refactor, and I've previously commented that I do not understand your vision of a refactor in this space, and certainly have not found capacity within my other FOSS commitments to to do a general code quality improvement here.

Since those comments, you announced "this looks reasonable". I'll have another very brief look at refactoring.

jnothman · 2017-10-16T01:35:30Z

I made the requested change as I interpret it. I do not find it improves code quality.

…er-styles

jnothman · 2017-10-29T10:18:40Z

I haven't understood the cause of the travis failure, apparently in lint.sh but without error message that I can see.

I've merged in master and moved what's new to the next version.

jreback · 2017-10-29T23:25:13Z

Linting *.py
pandas/io/excel.py:1794:1: E305 expected 2 blank lines after class or function definition, found 1

you can check this locally via:

make lint-diff (or just directly run flake8 on that file)

jnothman · 2017-10-29T23:35:37Z

Okay. I tried to run ci/lint.sh but got not flags. Thanks.

…

On 30 October 2017 at 10:25, Jeff Reback ***@***.***> wrote: Linting *.py pandas/io/excel.py:1794:1: E305 expected 2 blank lines after class or function definition, found 1 you can check this locally via: make lint-diff (or just directly run flake8 on that file) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#16149 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEz6yq4SKK71rbuRtXNVmBX6vxySOpjks5sxQllgaJpZM4NJIfp> .

TomAugspurger

The clipboard failure on travis looks unrelated.

All good @jreback?

jreback · 2017-10-31T00:34:25Z

doc/source/whatsnew/v0.22.0.txt

@@ -22,7 +22,7 @@ New features
 Other Enhancements
 ^^^^^^^^^^^^^^^^^^

-
+- Better support for ``Dataframe.style.to_excel()`` output with the ``xlsxwriter`` engine. (:issue:`16149`)


ok for now. it might make sense to enhance the excel docs with what is better now? (or maybe a listing of styles that work). but for a followup.

jreback · 2017-10-31T00:34:48Z

thanks @jnothman

jnothman force-pushed the xlsxwriter-styles branch 2 times, most recently from a3b2a87 to 3f687c4 Compare April 26, 2017 16:47

Support more styles for xlsxwriter

96ab259

jnothman force-pushed the xlsxwriter-styles branch from fd4050e to 96ab259 Compare April 27, 2017 07:20

jreback added the IO Excel read_excel, to_excel label Apr 27, 2017

jreback reviewed Apr 27, 2017

View reviewed changes

jnothman mentioned this pull request May 8, 2017

Use cssdecl package for resolving CSS #16170

Closed

4 tasks

jnothman added 2 commits June 14, 2017 13:34

Merge branch 'master' into xlsxwriter-styles

3515f6a

Move what's new to 0.21

740dca4

Merge branch 'master' into xlsxwriter-styles

9b0ea70

jreback requested changes Jun 28, 2017

View reviewed changes

jnothman added 2 commits August 17, 2017 20:49

Merge branch 'master' into xlsxwriter-styles

f413cb1

Merge remote-tracking branch 'origin/xlsxwriter-styles' into xlsxwrit…

ea3a468

…er-styles

jnothman commented Aug 17, 2017

View reviewed changes

ENH More efficient traversal of xlsxwriter styles where few types are…

30a8dc4

… in use

jnothman added 3 commits August 18, 2017 13:16

Merge branch 'master' into xlsxwriter-styles

1c4bcf9

Empty commit to restart build

38db7e6

Merge branch 'xlsxwriter-styles' of https://github.com/jnothman/pandas …

376bc8b

…into xlsxwriter-styles

jnothman added 2 commits October 16, 2017 12:08

Merge branch 'master' into xlsxwriter-styles

8d63f00

Factor out _XlsxStyler.convert method

d6441ff

jnothman added 3 commits October 16, 2017 12:35

Merge remote-tracking branch 'origin/xlsxwriter-styles' into xlsxwrit…

de808df

…er-styles

Merge branch 'master' into xlsxwriter-styles

06b1d7f

Move what's new to 0.22

26728ee

PEP8

80ed56a

TomAugspurger approved these changes Oct 30, 2017

View reviewed changes

jreback approved these changes Oct 31, 2017

View reviewed changes

jreback added this to the 0.22.0 milestone Oct 31, 2017

jreback reviewed Oct 31, 2017

View reviewed changes

jreback merged commit 5d096f7 into pandas-dev:master Oct 31, 2017

peterpanmj pushed a commit to peterpanmj/pandas that referenced this pull request Oct 31, 2017

Support more styles for xlsxwriter (pandas-dev#16149)

80b74a2

No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017

Support more styles for xlsxwriter (pandas-dev#16149)

4051b29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support more styles for xlsxwriter #16149

Support more styles for xlsxwriter #16149

jnothman commented Apr 26, 2017 •

edited

Loading

jnothman commented Apr 27, 2017

codecov bot commented Apr 27, 2017

codecov bot commented Apr 27, 2017 •

edited

Loading

jnothman commented Apr 27, 2017

jreback Apr 27, 2017

jnothman Apr 27, 2017

jreback Apr 27, 2017

jnothman Apr 27, 2017

jreback Apr 27, 2017

jnothman commented Apr 29, 2017

jreback commented Jun 10, 2017

jnothman commented Jun 14, 2017

jreback Jun 28, 2017

jnothman Aug 17, 2017

jnothman Aug 17, 2017

jreback Jun 28, 2017

jreback commented Aug 17, 2017

jnothman commented Aug 17, 2017

jnothman left a comment

jnothman Aug 17, 2017

jnothman Aug 17, 2017

jreback commented Aug 18, 2017

jnothman commented Oct 16, 2017

jreback commented Oct 16, 2017

jnothman commented Oct 16, 2017

jnothman commented Oct 16, 2017

jnothman commented Oct 29, 2017

jreback commented Oct 29, 2017

jnothman commented Oct 29, 2017 via email

TomAugspurger left a comment

jreback Oct 31, 2017

jreback commented Oct 31, 2017

Support more styles for xlsxwriter #16149

Support more styles for xlsxwriter #16149

Conversation

jnothman commented Apr 26, 2017 • edited Loading

jnothman commented Apr 27, 2017

codecov bot commented Apr 27, 2017

Codecov Report

codecov bot commented Apr 27, 2017 • edited Loading

Codecov Report

jnothman commented Apr 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jnothman commented Apr 29, 2017

jreback commented Jun 10, 2017

jnothman commented Jun 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Aug 17, 2017

jnothman commented Aug 17, 2017

jnothman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Aug 18, 2017

jnothman commented Oct 16, 2017

jreback commented Oct 16, 2017

jnothman commented Oct 16, 2017

jnothman commented Oct 16, 2017

jnothman commented Oct 29, 2017

jreback commented Oct 29, 2017

jnothman commented Oct 29, 2017 via email

TomAugspurger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Oct 31, 2017

jnothman commented Apr 26, 2017 •

edited

Loading

codecov bot commented Apr 27, 2017 •

edited

Loading