trivial (temporary?) fix for confusing list column printing, related,… #2562
Conversation
R/print.data.table.R
Outdated
| @@ -145,3 +145,6 @@ shouldPrint = function(x) { | |||
| # for removing the head (column names) of matrix output entirely, | |||
| # as opposed to printing a blank line, for excluding col.names per PR #1483 | |||
| cut_top = function(x) cat(capture.output(x)[-1L], sep = '\n') | |||
|
|
|||
| # FR #2591 - format.data.table issue with columns of class "formula" | |||
| is.formula <- function(x) class(x) == "formula" | |||
There was a problem hiding this comment.
^shouldn't this be inherits(x, 'formula')?
There was a problem hiding this comment.
Yes. Good spot. I'm about to merge so I'll do it quickly.
Codecov Report
@@ Coverage Diff @@
## master #2562 +/- ##
==========================================
+ Coverage 91.45% 91.59% +0.14%
==========================================
Files 63 63
Lines 12071 12184 +113
==========================================
+ Hits 11039 11160 +121
+ Misses 1032 1024 -8
Continue to review full report at Codecov.
|
…hould be minimal ideally) -- in this case, just the ... added is now more clear in the diff. Formatting changes can be made but if so, separately in a non-logic-change PR. If we elongated all tests like this then the lines of test code would expland dramatically. I also tend to press F5 in dev to run subsections of tests which is one reason I like dense.
|
Agree with adding |
|
all the same I think it's a holdover until we hash out the right way of incorporating |
|
Yep - |
|
Hmm, agree `","` is better. probably just write our own method.
…On Jan 10, 2018 8:34 PM, "Matt Dowle" ***@***.***> wrote:
Merged #2562 <#2562>.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2562 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AHQQdZKAQXi7guyceN-oDChQIAH07IV5ks5tJWUxgaJpZM4RaAtI>
.
|
… part of #1523
There's some more discussion on the merits of instead using
toStringhere which I tend to like, see the discussion on #1523; filing this PR to jumpstart that convo & offer a temporary fix in any case since this was trivial to do.Also brought
is.formulaintoprint.data.table.Rsince that's where it belongs. Side note -- shouldn't the function instead beinherits(x, 'formula')?