Integrate colformat #294

krlmlr · 2017-08-16T13:23:34Z

Closes #144. Closes #89.

krlmlr · 2017-08-16T13:24:18Z

tests/testthat/output/trunc_mat/POSIXlt-8-60.txt

-# ... with 4 more rows
+                    x y                  
+               <dttm> <dttm>             
+1 2016-01-01 12:34:57 2016-01-01 12:34:57


@hadley: How important is it that POSIXlt aren't shown in a tibble?

It's ok to show them, but they should be consistently aligned with POSIXct, and they need a different type_sum()

krlmlr · 2017-08-16T13:24:46Z

tests/testthat/output/trunc_mat/all--30.txt

@@ -1,10 +1,7 @@
 # A tibble: 3 x 9
-      a     b     c     d
+      a     b     c d    


@hadley: Does the alignment of the column titles look right to you?

I think the principle is to right align numbers and left align anything else. Currently logical is right aligned, but should probably be left aligned?

krlmlr · 2017-08-16T13:47:51Z

tests/testthat/output/trunc_mat/all--300.txt

-2   2.5     2 FALSE     b      b 2015-12-11 2015-12-09 10:51:36 <int [1]> <list [1]>
-3    NA    NA    NA  <NA>   <NA>         NA                  NA <int [1]> <list [1]>
+      a     b     c d     e                f                   g h         i         
+  <dbl> <int> <lgl> <chr> <fctr>      <date>              <dttm> <list>    <list>    


@hadley: Left-align list columns?

krlmlr · 2017-08-16T13:48:21Z

tests/testthat/output/trunc_mat/iris--70.txt

-10          4.9         3.1          1.5         0.1  setosa
-# ... with 140 more rows
+          <dbl>       <dbl>        <dbl>       <dbl> <fctr> 
+ 1         5.10        3.50         1.40       0.200 setosa 


@hadley: Left-align factors?

krlmlr · 2017-08-16T13:48:21Z

tests/testthat/output/trunc_mat/iris-3-5.txt

+1
+2
+3
+with 5 more variables: Sepal.Length <dbl>, Sepal.Width <dbl>, Petal.Length <dbl>, Petal.Width <dbl>, Species <fctr>


Need to respect width

krlmlr · 2017-08-16T13:49:10Z

tests/testthat/output/trunc_mat/long-5-30.txt

@@ -6,4 +6,3 @@
 3     3
 4     4
 5     5
-# ... with 9,995 more rows


Why is the number of missing rows not shown?

krlmlr · 2017-08-16T13:49:24Z

tests/testthat/output/trunc_mat/long_unk-5-30.txt

@@ -5,4 +5,4 @@
 3     3
 4     4
 5     5
-# ... with more rows
+with at least 5 rows total


What caused this change?

I bet this is an NA somewhere - that's the print behaviour for database tables where we don't know max

krlmlr · 2017-08-16T13:50:15Z

tests/testthat/output/trunc_mat/newline.txt

-1  "\n"   "\n"
-2  "\""   "\""
+1 \n    \n    
+2 "     "     


@hadley: Do we need to "-quote strings with special characters?

It's probably a good idea

krlmlr · 2017-08-16T13:50:15Z

tests/testthat/output/trunc_mat/wide-8-60.txt

@@ -1,5 +1,5 @@
 # A tibble: 3 x 2
-  成交日期 合同录入日期
+      成交日期       合同录入日期


Take care of wide characters in title.

krlmlr · 2017-08-16T13:50:43Z

tests/testthat/output/trunc_mat/zero_rows--30.txt

@@ -1,3 +1,3 @@
 # A tibble: 0 x 2
-# ... with 2 variables:
-#   a <chr>, b <lgl>
+  a         b


Don't show body for zero-row tibbles

krlmlr · 2017-08-16T19:13:44Z

tests/testthat/output/trunc_mat/POSIXlt-8-60.txt

-7 2016-01-01 12:35:03 <POSIXlt>
-8 2016-01-01 12:35:04 <POSIXlt>
+  x                   y                  
+  <dttm>              <ldttm>            


@hadley: Does "lddtm" work for POSIXlt?

I almost wonder if just "POSIXlt" would be best

krlmlr · 2017-08-16T19:14:50Z

tests/testthat/output/trunc_mat/iris-3-5.txt

-1          5.1
-2          4.9
-3          4.7
+


@hadley: Do we need to care about very small output widths (here: 5 characters)?

No. I think we're safe to assume a width of >20

hadley · 2017-08-16T20:09:24Z

tests/testthat/output/trunc_mat/all--30.txt

-3    NA    NA    NA  <NA>
+1  1.00     1 *     a    
+2  2.50     2 -     b    
+3 NA       NA ?     ?    


Shouldn't this NA be right aligned?

It's right aligned on decimal. This is colformat issue.

krlmlr · 2017-08-18T16:50:05Z

@jimhester: Any idea why colformat is not installed on Travis even if it is in the Remotes section? https://travis-ci.org/tidyverse/tibble/jobs/266023788#L2763

codecov · 2017-08-19T08:31:56Z

Codecov Report

Merging #294 into master will decrease coverage by 2.16%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #294      +/-   ##
==========================================
- Coverage   90.41%   88.24%   -2.17%     
==========================================
  Files          19       19              
  Lines         960      885      -75     
==========================================
- Hits          868      781      -87     
- Misses         92      104      +12

Impacted Files	Coverage Δ
R/tbl-df.r	`100% <ø> (ø)`	⬆️
R/type-sum.r	`77.77% <ø> (-22.23%)`	⬇️
R/utils-format.r	`94.21% <100%> (-5.31%)`	⬇️
R/compat-purrr.R	`14.08% <0%> (-1.41%)`	⬇️

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 3014d05...94bdb8b. Read the comment docs.

krlmlr · 2017-08-19T10:36:56Z

@jimhester: I suspect the reason was an invalid GITHUB_PAT env var, but I haven't found a log entry that hints at this problem -- only after changing to tic: https://travis-ci.org/tidyverse/tibble/builds/266228593#L900.

krlmlr · 2017-08-19T11:44:20Z

@hadley: Could you please take a final look at the output before I move the corresponding tests to colformat (#297)?

- Integrate colformat (#294).

integrate colformat

32092e0

krlmlr commented Aug 16, 2017

View reviewed changes

krlmlr added 12 commits August 16, 2017 20:18

remove tests now in colformat

93ccbaa

improve formatting

012532b

turn off kable for now

e5872f6

zero-row tibbles

d1837c3

fix wide characters

9d87639

left-align logical

ef1604f

footer with comment

a8f7d0c

oops

fc779e0

fix display of more rows

67fa52f

minor glitch

fc7962f

use squeeze()

ad6b30d

reenable knitr output

a7f7d76

krlmlr commented Aug 16, 2017

View reviewed changes

hadley reviewed Aug 16, 2017

View reviewed changes

krlmlr added 5 commits August 18, 2017 17:02

knit_print() support

df5ed8c

logical format

51a268e

strings use NA if color is not available

1c3ecaa

oops

43022e5

reexport from colformat

7a3713e

krlmlr added 2 commits August 19, 2017 10:08

use tic

eab8e76

document

6c05e64

krlmlr added 2 commits August 19, 2017 13:30

fix bogus warning

06c1406

Merge remote-tracking branch 'origin/master' into f-colformat

a4e1c80

krlmlr changed the title ~~WIP: Integrate colformat~~ Integrate colformat Aug 19, 2017

krlmlr requested a review from hadley August 19, 2017 11:43

hadley approved these changes Aug 21, 2017

View reviewed changes

krlmlr added 4 commits August 22, 2017 09:19

Merge branch 'master' into f-colformat

a897b92

extract functions

6bac30e

extract variable

4ae470d

remove colformat output from output tests

bb20fa7

krlmlr mentioned this pull request Aug 25, 2017

Prepare for tibble integration r-lib/pillar#33

Merged

tests moved to colformat

94bdb8b

krlmlr merged commit 49c4d6c into master Aug 25, 2017

krlmlr deleted the f-colformat branch August 25, 2017 21:00

krlmlr added a commit that referenced this pull request Aug 25, 2017

Merge tag 'v1.3.4.9001'

503d7f0

- Integrate colformat (#294).

github-actions bot locked as resolved and limited conversation to collaborators Dec 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate colformat #294

Integrate colformat #294

krlmlr commented Aug 16, 2017 •

edited

Loading

krlmlr Aug 16, 2017

hadley Aug 16, 2017

krlmlr Aug 16, 2017

hadley Aug 16, 2017

krlmlr Aug 16, 2017

krlmlr Aug 16, 2017

krlmlr Aug 16, 2017

krlmlr Aug 16, 2017

krlmlr Aug 16, 2017

hadley Aug 16, 2017

krlmlr Aug 16, 2017

hadley Aug 16, 2017

krlmlr Aug 16, 2017

krlmlr Aug 16, 2017

krlmlr Aug 16, 2017

hadley Aug 16, 2017

krlmlr Aug 16, 2017

hadley Aug 16, 2017

hadley Aug 16, 2017

hadley Aug 16, 2017

krlmlr commented Aug 18, 2017

codecov bot commented Aug 19, 2017 •

edited

Loading

krlmlr commented Aug 19, 2017

krlmlr commented Aug 19, 2017

-5.1
-4.9
-4.7

Integrate colformat #294

Integrate colformat #294

Conversation

krlmlr commented Aug 16, 2017 • edited Loading

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

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

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

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

krlmlr commented Aug 18, 2017

codecov bot commented Aug 19, 2017 • edited Loading

Codecov Report

krlmlr commented Aug 19, 2017

krlmlr commented Aug 19, 2017

krlmlr commented Aug 16, 2017 •

edited

Loading

codecov bot commented Aug 19, 2017 •

edited

Loading