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.

Sign up for GitHub

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

Create print.one_skim_df, document reassign_skim_attrs and modify re… #703

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Create print.one_skim_df, document reassign_skim_attrs and modify re… #703

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
562b753
Create print.one_skim_df, document reassign_skim_attrs and modify re…
elinw Apr 26, 2022
255b760
Merge branch 'main' into r4_2
elinw Oct 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NAMESPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ S3method(knit_print,skim_df)
S3method(knit_print,skim_list)
S3method(knit_print,summary_skim_df)
S3method(mutate,skim_df)
S3method(print,one_skim_df)
S3method(print,skim_df)
S3method(print,skim_list)
S3method(print,summary_skim_df)
Expand Down
2 changes: 1 addition & 1 deletion R/reshape.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ reconcile_skimmers <- function(data, groups, base) {
)
matched_cols <- dplyr::intersect(all_columns, with_base_columns)
extra_cols <- dplyr::setdiff(all_columns, with_base_columns)
if (length(extra_cols) > 0) {
if (length(extra_cols) > 0 && "skim_type" %in% names(data)) {
grouped <- dplyr::group_by(data, .data$skim_type)
complete_by_type <- dplyr::summarize_at(
grouped,
Expand Down
7 changes: 5 additions & 2 deletions R/skim_obj.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,12 @@ strip_skim_attrs <- function(object) {
attributes(object) <- stripped
object
}

#' @param object a skim object
#' @param skim_df name of new skim object
#' @param ... additional options
#'
#' Pass attributes from a `skimr` object to a new object.
#' @noRd
Copy link
Collaborator

@michaelquinn32 michaelquinn32 Apr 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that this change accomplishes anything. For one, I don't see a documentation file generated to match this function. That's a good thing. Documenting this function (removing the noRd tag) is an issue because it is not exported.

Do you mind reverting this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what is causing one of the errors when it builds.

#' @keywords internal
reassign_skim_attrs <- function(object, skim_df, ...) {
defaults <- list(
class = c("skim_df", "tbl_df", "tbl", "data.frame"),
Expand Down
21 changes: 21 additions & 0 deletions R/skim_print.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,27 @@ print.skim_df <- function(x,
}
}

#' @export
print.one_skim_df <- function(x,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have a summary method for a one_skim_df. Can we remove the logic for the summary and instead have this?

print.one_skim_df <- function(x, n, width, ...) {
  NextMethod("print")
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the logic of

skimr/R/skim_print.R

Line 38 in 562b753

print.skim_df <- function(x,

but took out the purrr.

I'm not sure but now that we are not silently getting the warning it may be that other parts are getting hit in new ways.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just make include_summary = FALSE ?

include_summary = TRUE,
n = Inf,
width = Inf,
summary_rule_width = getOption(
"skimr_summary_rule_width",
default = 40
),
...) {
if (is_skim_df(x) && nrow(x) > 0) {
if (include_summary) {
print(summary(x), .summary_rule_width = summary_rule_width, ...)
}

print(x, width = width, n = n, ...)
} else {
NextMethod("print")
}
}

# Methods for correctly formatting a a `one_skim_df`. We leverage the
# customiztion options in `pillar` for this. It divides the results into: a
# header, which we customize; a body, where we strip some values; and a footer,
Expand Down