Skip to content

[feat] add options to scale and round table values#263

Open
Schiano-NOAA wants to merge 3 commits into
mainfrom
feat-scale-tbls
Open

[feat] add options to scale and round table values#263
Schiano-NOAA wants to merge 3 commits into
mainfrom
feat-scale-tbls

Conversation

@Schiano-NOAA
Copy link
Copy Markdown
Collaborator

@Schiano-NOAA Schiano-NOAA commented May 19, 2026

Addresses suggestions from workshops and issues #254 & #255

@github-actions
Copy link
Copy Markdown
Contributor

New version checklist

  • Package version in DESCRIPTION has been updated
  • Release notes have been drafted/published
  • Cheatsheet content has been updated (if applicable)
  • Cheatsheet version has been updated

@github-actions
Copy link
Copy Markdown
Contributor

Code Metrics Report

Coverage Code to Test Ratio Test Execution Time
66.2% 1:0.1 5m7s

Code coverage of files in pull request scope (78.6%)

Files Coverage
R/table_landings.R 95.7%
R/utils_table.R 71.0%

Reported by octocov

Comment thread R/table_landings.R
#' want to just summarize the data across all factors, set group = "none".
#' @param method A string describing the method of summarizing data when group
#' is set to "none". Options are "sum" or "mean". Default is "sum".
#' @param digits Numeric value indicating the number of rounding digits.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#' @param digits Numeric value indicating the number of rounding digits.
#' @param digits Numeric value indicating the number of rounding digits. Trailing zeroes will not be added to the estimate (example: rounding 0.1 to 4 digits will still produce 0.1).

This doesn't add trailing zeroes if the number doesn't have them (example: round(as.numeric(5.5), digits = 4) will still produce 5.5. I suggest noting that here

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I wonder if there is a arg in round to still show trailing zeros. I am going to look into it before I merge this suggestion.

Copy link
Copy Markdown
Collaborator

@sbreitbart-NOAA sbreitbart-NOAA left a comment

Choose a reason for hiding this comment

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

I had one minor suggestion. Also, did you mean to reference this issue in the description? (254 is repeated): #255

Otherwise, looks good! Minor changes but substantial benefits for users!

@Schiano-NOAA
Copy link
Copy Markdown
Collaborator Author

I had one minor suggestion. Also, did you mean to reference this issue in the description? (254 is repeated): #255

Otherwise, looks good! Minor changes but substantial benefits for users!

LOL yes it was supposed to be 255 as well. I will make those changes then merge! Thanks for the review 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feat] add argument to scale values in tables [Feat] Add argument to round values to x decimal

2 participants