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

Add color/formatting to output of print methods #124

Closed
zachary-foster opened this issue Jan 24, 2018 · 14 comments
Closed

Add color/formatting to output of print methods #124

zachary-foster opened this issue Jan 24, 2018 · 14 comments
Milestone

Comments

@zachary-foster
Copy link
Collaborator

@zachary-foster zachary-foster commented Jan 24, 2018

@sckott, what do you think of adding color/formatting to the output of print methods using the crayon package? Do have any experience with this? Do you know of potential problems with consoles that do not understand the color codes?

I was trying to figure out how the tibbles are color coded now, but I don't see that code in the tibble package even though it imports crayon.

@sckott
Copy link
Member

@sckott sckott commented Jan 24, 2018

sounds good to me. not sure how it works - we should do it if possible

@zachary-foster
Copy link
Collaborator Author

@zachary-foster zachary-foster commented Jan 24, 2018

Cool, I made a few things bold already. Its really easy, but I don't know if it will introduce edge case problems

zachary-foster added a commit that referenced this issue Jan 24, 2018
@sckott
Copy link
Member

@sckott sckott commented Jan 31, 2018

cool to see the colors 🌈

@zachary-foster
Copy link
Collaborator Author

@zachary-foster zachary-foster commented Jan 31, 2018

Nice! I think the colors will help with these complex print methods. Feel free to change the specific styles here:

taxa/R/taxmap--printers.R

Lines 357 to 413 in a7ac6b3

#' Taxon id formatting in print methods
#'
#' A simple wrapper to make changing the formatting of text printed easier.
#'
#' @param text What to print
#'
#' @family printer fonts
#'
#' @keywords internal
tid_font <- function(text) {
crayon::green(text)
}
#' Punctuation formatting in print methods
#'
#' A simple wrapper to make changing the formatting of text printed easier.
#' This is used for non-data, formatting characters
#'
#' @param text What to print
#'
#' @family printer fonts
#'
#' @keywords internal
punc_font <- function(text) {
crayon::silver(text)
}
#' Descripton formatting in print methods
#'
#' A simple wrapper to make changing the formatting of text printed easier.
#' This is used for non-data, formatting characters
#'
#' @param text What to print
#'
#' @family printer fonts
#'
#' @keywords internal
desc_font <- function(text) {
crayon::italic(text)
}
#' Variable name formatting in print methods
#'
#' A simple wrapper to make changing the formatting of text printed easier.
#' This is used for non-data, formatting characters
#'
#' @param text What to print
#'
#' @family printer fonts
#'
#' @keywords internal
name_font <- function(text) {
crayon::bold(text)
}

I am still not sure about the best styles to use for each part

@sckott
Copy link
Member

@sckott sckott commented Jan 31, 2018

I'm not sure what's best either. maybe they have best practices docs somehwere?

@zachary-foster
Copy link
Collaborator Author

@zachary-foster zachary-foster commented Feb 8, 2018

I searched for best practices after you suggested it and did not find any. Currently, my strategy is to copy the tibble print methods. Unfortunately, I cant seem to find a way to print a indented tibble with colors. That would be nice for displaying tibbles stored in taxmap objects.

@sckott
Copy link
Member

@sckott sckott commented Feb 9, 2018

okay. i guess if can't indent the table, just a newline or two to set it off visually?

@zachary-foster
Copy link
Collaborator Author

@zachary-foster zachary-foster commented Feb 9, 2018

That would work. Although I can get them to indent without color using :

#' Print a object with a prefix
#'
#' Print a object with a prefix. Uses the standard print method of the object.
#'
#' @param x What to print.
#'
#' @keywords internal
prefixed_print <- function(x, prefix, ...) {
output <- paste0(prefix, utils::capture.output(print(x, ...)))
cat(paste0(paste0(output, collapse = "\n"), "\n"))
}

I then was trying to add some color back, but I doubt it would be worth the trouble to completely replicate the color. I mostly just want the taxon IDs highlighted. I managed to highlight the taxon ID column name, so thats something at least.

@zachary-foster
Copy link
Collaborator Author

@zachary-foster zachary-foster commented Feb 9, 2018

It would be especially cool if only valid taxon IDs were colored as taxon IDs so people can tell quickly which datasets are mapped to taxa and if there is anything wrong with the IDs (e.g. a taxonless data after filtering with drop_taxa = FALSE). However, I am not sure if it would be too much of a performance cost to check if IDs are valid every time a big object is printed.

@sckott
Copy link
Member

@sckott sckott commented Feb 9, 2018

valid in what sense?

@zachary-foster
Copy link
Collaborator Author

@zachary-foster zachary-foster commented Feb 9, 2018

All should be either an ID in the edge list / names(obj$taxa) or NA.

@sckott
Copy link
Member

@sckott sckott commented Feb 9, 2018

ah, okay, yeah, seems like it'd be fast enough

@zachary-foster
Copy link
Collaborator Author

@zachary-foster zachary-foster commented Feb 9, 2018

Ok cool

@zachary-foster
Copy link
Collaborator Author

@zachary-foster zachary-foster commented Mar 7, 2018

Seems to be working now. Things now only get the "taxon ID" green font if the are all valid taxon ids. If there is a "taxon_id" column, but there are invalid ids, then the column header gets an angry red color.

@zachary-foster zachary-foster mentioned this issue Apr 5, 2018
3 of 3 tasks complete
@zachary-foster zachary-foster added this to the v0.2.1 milestone Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.