Rapporter / pander

An R Pandoc Writer: Convert arbitrary R objects into markdown
http://rapporter.github.io/pander/
Open Software License 3.0
294 stars 66 forks source link

Moved calls to 'to.plain.ascii' before the column widths are calculated (fix for issue #321) #330

Closed dcomtois closed 5 years ago

dcomtois commented 5 years ago

Ran all tests on Linux and Windows and no detectable errors are introduced.

In the example I gave, when I opened the issue, we had:

pander(obj, style = "multiline", plain.ascii = TRUE, justify = "left", 
       keep.line.breaks = TRUE, split.tables = Inf, split.cells = Inf)

# -----------------------------------------------------------------------
# Variable    Text Graph                              Valid     Missing  
# ----------- --------------------------------------- --------- ---------
# age         .     .         .   . :                 975       25       
# [numeric]   : . . : : :   : : . : :                 (97.5%)   (2.5%)   
#             : : : : : : : : : : : :                                    
#             : : : : : : : : : : : :                                    
#             : : : : : : : : : : : :                                    
#             : : : : : : : : : : : :                                    
# -----------------------------------------------------------------------

After the change, we have:

# ---------------------------------------------------------
# Variable    Text Graph                Valid     Missing  
# ----------- ------------------------- --------- ---------
# age         .     .         .   . :   975       25       
# [numeric]   : . . : : :   : : . : :   (97.5%)   (2.5%)   
#             : : : : : : : : : : : :                      
#             : : : : : : : : : : : :                      
#             : : : : : : : : : : : :                      
#             : : : : : : : : : : : :                      
# ---------------------------------------------------------

Note: The only failing test that I noticed had nothing to do with this p.r. -- it's in the helpers section, 'splitLine with hyphening' and was already present.

dcomtois commented 5 years ago

Let me check what this failing test is about... Surprisingly it didn't show up when I ran the tests myself.

dcomtois commented 5 years ago

Okay so I left the handling of rownames and colnames as they were, but did move up the call to to.plain.ascii for the values (t) before calculation of column widths. Not ideal but since I can't build locally (ref: issue #319), debugging is very tedious. I think this is at least safe and functional.

codecov-io commented 5 years ago

Codecov Report

Merging #330 into master will increase coverage by <.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #330      +/-   ##
==========================================
+ Coverage    78.8%   78.81%   +<.01%     
==========================================
  Files          12       12              
  Lines        3109     3110       +1     
==========================================
+ Hits         2450     2451       +1     
  Misses        659      659
Impacted Files Coverage Δ
R/pandoc.R 87.06% <100%> (+0.02%) :arrow_up:

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 ca3ecf6...6840e96. Read the comment docs.

daroczig commented 5 years ago

thanks :+1: