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

Use tibble::print.tbl() for printing #14

Merged
merged 4 commits into from May 15, 2017

Conversation

@krlmlr
Copy link
Member

krlmlr commented May 13, 2017

now only need to override tbl_sum(). Also fixes tests.

Closes tidyverse/dplyr#2749.

@hadley: Could you please review (and enable reviews for this repo)?

@krlmlr krlmlr force-pushed the krlmlr:f-#2749-print-tbl branch from 3b19dbe to 388c001 May 13, 2017
@hadley

This comment has been minimized.

Copy link
Member

hadley commented May 13, 2017

Can you keep the extra spaces so the colons line up?

@krlmlr krlmlr mentioned this pull request May 13, 2017

# S3: tbl_dbi
# A tibble : ?? x 5
# Source : table<iris-output-test> [?? x 5]

This comment has been minimized.

Copy link
@krlmlr

krlmlr May 14, 2017

Author Member

Now aligning colons by adding space before them. Let me know if you prefer adding space after the colon, like in the original output.

This comment has been minimized.

Copy link
@hadley

hadley May 15, 2017

Member

I prefer space after colon


# S3: tbl_dbi
# A tibble : ?? x 5
# Source : table<iris-output-test> [?? x 5]

This comment has been minimized.

Copy link
@krlmlr

krlmlr May 14, 2017

Author Member

Now aligning colons by adding space before them. Let me know if you prefer adding space after the colon, like in the original output.

Ordered by: Sepal.Length

# S3: tbl_dbi
# A tibble : ?? x 5

This comment has been minimized.

Copy link
@krlmlr

krlmlr May 14, 2017

Author Member

Do we need to keep this?

This comment has been minimized.

Copy link
@hadley

hadley May 15, 2017

Member

I don't think so

@hadley: Please review
@hadley

This comment has been minimized.

Copy link

hadley commented on 267d7b7 May 15, 2017

Looks great!

@krlmlr

This comment has been minimized.

Copy link
Member Author

krlmlr commented May 15, 2017

I don't have permission to merge.

@hadley hadley merged commit edede07 into tidyverse:master May 15, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@javierluraschi

This comment has been minimized.

Copy link
Contributor

javierluraschi commented Aug 1, 2017

@krlmlr See tidyverse/tibble#285, dplyr used to trigger tbl_desc() for computing the headers of a tibble, but now we rely on tibble itself, which is great; however, tibble calls nrow and dim which is subclassed in some backends to trigger a full table scan.

@krlmlr krlmlr deleted the krlmlr:f-#2749-print-tbl branch Mar 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.