Skip to content
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

Don't add rowspan/colspan if it's 1. #15403

Closed
wants to merge 1 commit into from

Conversation

QuLogic
Copy link
Contributor

@QuLogic QuLogic commented Feb 14, 2017

Just a small thing I noticed in a footnote here. Probably can't do much about the extra classes, but rowspan/colspan seem like easy fixes to save a few bytes per row/col and it's already done in the other code path.

Don't really know if this is major enough to deserve a what's new entry.

  • closes #xxxx - N/A
  • tests added / passed
  • passes git diff upstream/master | flake8 --diff
  • whatsnew entry

@codecov-io
Copy link

codecov-io commented Feb 15, 2017

Codecov Report

Merging #15403 into master will decrease coverage by -0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #15403      +/-   ##
==========================================
- Coverage   90.37%   90.37%   -0.01%     
==========================================
  Files         135      135              
  Lines       49454    49463       +9     
==========================================
+ Hits        44694    44702       +8     
- Misses       4760     4761       +1
Impacted Files Coverage Δ
pandas/formats/style.py 96.17% <100%> (+0.11%)
pandas/core/common.py 91.02% <ø> (-0.34%)

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 9b5d848...9a8fcee. Read the comment docs.

@jreback jreback added IO HTML read_html, to_html, Styler.apply, Styler.applymap Output-Formatting __repr__ of pandas objects, to_string labels Feb 15, 2017
@TomAugspurger
Copy link
Contributor

@QuLogic thanks. Could you check and see if we have any tests of colspan > 1 when there's a MultiIndex in the columns? I see we have one for rowspan > 1 with a MI in the index, but didn't see one for the columns.

@QuLogic
Copy link
Contributor Author

QuLogic commented Feb 15, 2017

I'm not sure where it is, but codecov says it's hitting both colspan > 1 and rowspan > 1.

@jreback
Copy link
Contributor

jreback commented Feb 16, 2017

is there any visible effect of this? You can add a whatsnew entry with this PR number just to document. Maybe under Other Enhancements. lgtm. otherwise.

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Feb 16, 2017 via email

@jreback jreback added this to the 0.20.0 milestone Feb 16, 2017
Save a few bytes per row/col and it's already done in the other code
path.
@jreback
Copy link
Contributor

jreback commented Feb 17, 2017

thanks @QuLogic

@jreback jreback closed this in f65a641 Feb 17, 2017
@QuLogic QuLogic deleted the no-extra-span branch February 18, 2017 08:42
AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this pull request Mar 21, 2017
Just a small thing I noticed in a [footnote
here](https://danluu.com/web-bloat/#appendix-irony). Probably can't do
much about the extra classes, but rowspan/colspan seem like easy fixes
to save a few bytes per row/col and it's already done in the other
code path.

Author: Elliott Sales de Andrade <quantum.analyst@gmail.com>

Closes pandas-dev#15403 from QuLogic/no-extra-span and squashes the following commits:

9a8fcee [Elliott Sales de Andrade] Don't add rowspan/colspan if it's 1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IO HTML read_html, to_html, Styler.apply, Styler.applymap Output-Formatting __repr__ of pandas objects, to_string
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants