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

Print the number of variables in glimpse. #988

Merged
merged 7 commits into from Aug 26, 2015
Merged

Conversation

@ilarischeinin
Copy link
Contributor

@ilarischeinin ilarischeinin commented Feb 19, 2015

Adds the number of variables to the output of glimpse. Also, wherever thousand separators are used, use "," only if it isn't already being used for the decimal point, in which case use ".".

…nd variables. Also, wherever thousand separators are used, use "," only if it isn't already being used for the decimal point, in which case use ".".
@coveralls
Copy link

@coveralls coveralls commented Feb 19, 2015

Coverage Status

Coverage decreased (-0.03%) to 67.0% when pulling 39dde9c on ilarischeinin:master into d78f5ca on hadley:master.

@hadley
Copy link
Member

@hadley hadley commented Aug 24, 2015

Good idea overall, but can you please pull out the comma/period-adding code into its own function to avoid the duplication?

@ilarischeinin
Copy link
Contributor Author

@ilarischeinin ilarischeinin commented Aug 25, 2015

Good point. I made it a function called big_mark(), which I put at the end of file utils-format.r (not sure if that's the best file to include it in though).


# function for the thousand separator,
# returns "," unless it's used for the decimal point, in which case returns "."
big_mark <- function() {

This comment has been minimized.

@hadley

hadley Aug 25, 2015
Member

I think you can reduce the duplication even further if big_mark() looked like this:

big_mark <- function(x, ...) { 
  mark <- if (identical(getOption("OutDec"), ",") "." else ","
  format(d, big.mark = mark, ...)
}

Note that here it's better to use non-vectorised equals (i.e. identical()) and ifelse() (i.e. if)

@ilarischeinin
Copy link
Contributor Author

@ilarischeinin ilarischeinin commented Aug 25, 2015

True. Earlier I made it separate so that big_mark() could be used with format(), formatC(), and prettyNum(). But the three of them are a bit redundant I guess.

@@ -25,7 +25,7 @@ NULL
#' @rdname dplyr-formatting
dim_desc <- function(x) {
d <- dim(x)
d2 <- format(d, big.mark = ",", justify = "none", trim = TRUE)
d2 <- big_mark(d)

This comment has been minimized.

@hadley

hadley Aug 25, 2015
Member

Do you no longer need the other args?

This comment has been minimized.

@ilarischeinin

ilarischeinin Aug 26, 2015
Author Contributor

No, as I used formatC() instead of format(). (Because it seems to be slightly more efficient.)

But actually, while it makes no difference here, for glimpse() there is a small cosmetic choice to be made. With the code above (that uses formatC()), its output currently looks like this:

> glimpse(flights)
Observations: 336,776
Variables: 16
$ year      (int) 2013, 2013, 2013, 2013, 2013, 2013, 2013, 2013, 2013, 201...
$ month     (int) 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, ...

But with format() it would be possible to align the two numbers like this:

> glimpse(flights)
Observations: 336,776
Variables:         16
$ year      (int) 2013, 2013, 2013, 2013, 2013, 2013, 2013, 2013, 2013, 201...
$ month     (int) 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, ...

The reason I didn't align them is that I'd expect the number of variables to practically always be way way smaller than the number of observations (and to very very rarely actually use the thousands separator).

But if you'd prefer the aligned version, I'll change big_mark() from formatC() to format(), which will by default produce right-aligned output. Then the function call inside dim_desc() would also get back its trim=TRUE argument so that it'll continue to print

Source: local data frame [336,776 x 16]

instead of

Source: local data frame [336,776 x       16]

This comment has been minimized.

@hadley

hadley Aug 26, 2015
Member

Looks good as is - thanks for the explanation!

@hadley
Copy link
Member

@hadley hadley commented Aug 26, 2015

One last thing - can you please add a bullet point to NEWS? It should look like * description (@yourusername, #thisprnumber)

@ilarischeinin
Copy link
Contributor Author

@ilarischeinin ilarischeinin commented Aug 26, 2015

There you go, I added it as two separate bullet point entries.

hadley added a commit that referenced this pull request Aug 26, 2015
Print the number of variables in glimpse.
@hadley hadley merged commit c7b9993 into tidyverse:master Aug 26, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hadley
Copy link
Member

@hadley hadley commented Aug 26, 2015

Thanks!

krlmlr pushed a commit to krlmlr/dplyr that referenced this pull request Mar 2, 2016
Print the number of variables in glimpse.
@lock
Copy link

@lock lock bot commented Jan 19, 2019

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants