Fix Excel-specific border styles

[tehunter]

• closes BUG: df.to_excel fails for "border-style: hair" with xlsxwriter #48649
• Tests added and passed if fixing a bug or adding a new feature
• All code checks passed.
• Added type annotations to new arguments/methods/functions.
• Added an entry in the latest doc/source/whatsnew/v1.5.1.rst doc/source/whatsnew/v1.5.2.rst file if fixing a bug or adding a new feature.

Fixes Excel-specific border styles and modifies behavior when border-style provided without a border-color to match CSS behavior (i.e., defaults to black)

[tehunter]

Let me know if the behavior change for unspecified colors is too much for a minor version. The new behavior matches CSS behavior so it follows the more expected behavior and will have more consistency between styler.to_html() and styler.to_excel() results.

[mroeschke]

I would slightly learn toward moving this to v1.6.0.rst as it appears this was "unsupported" rather than "use to work"

[tehunter]

I could support either, but to play devil's advocate: pandas never specified that these particular border-styles were supported or unsupported. The closest thing to documentation was this comment block in _border_style which implies there is a path to getting the "hair" style (as well as "mediumDashDot", "slantDashDot", "dashDot", and "dashDotDot") when one didn't actually exist:

        # convert styles and widths to openxml, one of:
        #       'dashDot'
        #       'dashDotDot'
        #       'dashed'
        #       'dotted'
        #       'double'
        #       'hair'
        #       'medium'
        #       'mediumDashDot'
        #       'mediumDashDotDot'
        #       'mediumDashed'
        #       'slantDashDot'
        #       'thick'
        #       'thin'

I think the error when using xlsxwriter with an unsupported border-style should be fixed in v1.5.1 (i.e. return "none" at end of function), as the stack location and error message make it very unclear where the user has introduced the error. And if we're fixing that, then it's pretty small step from there to add the support for the other styles.

I definitely think I'm onboard with removing the color defaulting portion from a v1.5.1 and moving it to a separate PR for v1.6.0

[rhshadrach]

I think the error when using xlsxwriter with an unsupported border-style should be fixed in v1.5.1 (i.e. return "none" at end of function), as the stack location and error message make it very unclear where the user has introduced the error.

The criteria for whether something should go in a patch or minor version is "did it work before". That is, patch releases should only fix regressions. As this isn't a regression, I'm also +1 on putting this in 1.6.0.

[attack68]

I suggest that if border-style attribute was supported for some styles, in v1.5.0, one should, by extension, assume that all the different border-style values were supported without their explicit documentation. And a failing to not support certain values is either a bug in the code or a bug in the documentation. I support 1.5.1.

[mroeschke]

Okay given the reduced scope (removing the color defaulting). I would be okay with 1.5.1

[rhshadrach]

Looks great!

[attack68]

lgtm

[tehunter]

Caught and fixed a bug when using these styles in the border shorthand (e.g. border-left: black 1px mediumDashDot). I think it's good to go now @attack68

[attack68]

looks good, can you merge main

[tehunter]

@attack68 main merged and tests pass

[attack68]

@rhshadrach merging is blocked here. are we missing unaddressed requests?

also is the pull request reflecting the right branch. im not up to speed with current 2.0 or 1.5.2 route?

Changes made

[rhshadrach]

@rhshadrach merging is blocked here. are we missing unaddressed requests?

Thanks for the ping - I've dismissed my stale review as the changes were made, so it's no longer blocking.

also is the pull request reflecting the right branch. im not up to speed with current 2.0 or 1.5.2 route?

In either case, we'll merge into main first and then backport if it should go into 1.5.2. As long as it's tagged with the 1.5.2 milestone, the backport will be attempted by meeseeks. The discussion of which to do is here: #48660 (comment)

[attack68]

@mroeschke are you ok with this? If so I think its OK to merge.

[attack68]

It seems 1.5.2 si ready for release, this seems to have been pushed to 1.5.3, can you update the whatrs new

[tehunter]

@attack68 Merged from main and moved the whatsnew entry to 1.5.3. It should be good to go assuming tests pass

[attack68]

@mroeschke you happy also if we merge this?

[mroeschke]

Sorry for the delay on my end, thanks @tehunter for sticking with it

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

git checkout 1.5.x
git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 d52b6b334677aad76334b5246ce26728156c0d70

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #48660: Fix Excel-specific border styles'

4. Push to a named branch:

git push YOURFORK 1.5.x:auto-backport-of-pr-48660-on-1.5.x

5. Create a PR against branch 1.5.x, I would have named this PR:

"Backport PR #48660 on branch 1.5.x (Fix Excel-specific border styles)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

[mroeschke]

@tehunter or @attack68 do either of you have availability to follow the back porting instructions above?

[attack68]

#50204