436 fr function format sigfig j by iaugusty · Pull Request #437 · johnsonandjohnson/junco

iaugusty · 2026-08-18T14:05:55Z

Pull Request

Fixes #436

Checks

(Have you updated the NEWS.md ?)
(Have you added proper tests for new functions/features ?)
(Have you added new functions to the pkgdown.yml ?)
(Have you run document() on new functions ?)

wwojciech

Overall, the idea looks good to me. Nevertheless, it would be good to clarify and potentially improve some of the design assumptions and implementation details, as indicated in my comments.

Please also check whether the CI/CD failure is related to the new code.

wwojciech · 2026-08-22T08:31:46Z

+
+#' @return numeric vector of the same length as `x`, rounded to `digits` significant figures.
+#' @keywords internal
+signif_j <- function(x, digits = 6, round_type = valid_round_type, whole_integer = FALSE, zero_threshold = 0) {


I'd aim for the most straightforward extension of base::signif(): preserve its behavior while allowing custom rounding, including the support for round_fun that does not support negative digits; and an optional zero_threshold.

I propose:

signif_j <- function( x, digits = 6, zero_threshold = 0, round_fun = round_fmt, ... )

Here, ... are passed as arguments to round_fun.

munoztd0 · 2026-08-24T12:41:08Z

Overall, the idea looks good to me. Nevertheless, it would be good to clarify and potentially improve some of the design assumptions and implementation details, as indicated in my comments.

Please also check whether the CI/CD failure is related to the new code.

CI/CD issues are not related to us, just ubuntu stuff, will resolveshortly

wwojciech

A few areas should be strengthened:

Add argument assertions for modified_signif_j() (specifically zero_threshold should be very well-assserted).
Improve documentation for modified_signif_j():

describe in detail how this function differs from base::signif().
add illustrative examples
use lifecycle badge.

Prepare dedicated unit test for various values of the arguments.

Please use mantel_fleiss_crit() ( pharmaverse/tern#1513 ) as a reference.

wwojciech

Changes required.

wwojciech · 2026-08-29T07:25:27Z

@@ -384,6 +390,8 @@
 modified_signif_j <- function(x, digits = 6, round_type = valid_round_type, whole_integer = FALSE, zero_threshold = 0) {
  stopifnot(length(digits) == 1, is.numeric(digits))


Use checkmate instead.

wwojciech · 2026-08-29T07:26:47Z

 modified_signif_j <- function(x, digits = 6, round_type = valid_round_type, whole_integer = FALSE, zero_threshold = 0) {
  stopifnot(length(digits) == 1, is.numeric(digits))

+  checkmate::assert_numeric(zero_threshold, lower = 0, upper = 10^(-digits), len = 1)


is missing value allowed? I guess not - use assert_number() instead.
How about other args, why are they not asserted?

wwojciech · 2026-08-29T07:28:30Z

  expect_snapshot(cran = TRUE, rslt)
 })
+
+test_that("explicit modified_signif_j tests", {


Tests should be specific: 1 test = 1 issue (or a few very similar issues). Please avoid putting all the checks into 1 tests. Each test should focus on a specific functionality or scenario so that it is clear what exactly is being verified.

wwojciech · 2026-08-29T07:29:16Z

 #' my_range_format(c(0.35235, 99.2342, 1, 1))
 #' my_range_format <- jjcsformat_range_fct("xx.xx", censor_char = "*")
 #' my_range_format(c(0.35235, 99.2342, 1, 1))
 jjcsformat_range_fct <- function(str, censor_char = "+") {


why is str not asserted?

wwojciech · 2026-08-29T07:30:55Z

 #' @noRd
-#'
+#' @description `r lifecycle::badge('stable')`
+#' Modified version of base function `signif` on 3 topics.


Modified version of [base::signif()] with three extensions:

wwojciech · 2026-08-29T07:34:04Z

+#' Very small absolute values can be considered zero (`zero_threshold`).\cr
+#' Showing all significant figures to the left of the decimal marker (`whole_integer`)
+#' rather than the specified number of significant figures.


#' Very small absolute values can be treated as zero (zero_threshold).
#' All significant figures to the left of the decimal marker can be shown (whole_integer), rather than limiting the result to the specified number of significant figures.

Please use roxygen list.

wwojciech · 2026-08-29T07:35:56Z

+#' Very small absolute values can be considered zero (`zero_threshold`).\cr
+#' Showing all significant figures to the left of the decimal marker (`whole_integer`)
+#' rather than the specified number of significant figures.
 #' @param x (`numeric`)\cr numeric vector to round.


Should start with upper case.
I think there is not need to repeat "numeric vector" - this is already clear from (numeric)\cr.
"Numbers to round." - should be ok.

wwojciech · 2026-08-29T07:37:22Z

+#' rather than the specified number of significant figures.
 #' @param x (`numeric`)\cr numeric vector to round.
 #' @param digits (`integer(1)`)\cr number of significant figures to display.
 #' @param round_type (`character(1)`)\cr rounding method. See [formatters::format_value()] for details.


Add info that this arg is passed directly to [formatters::round_fmt()]

wwojciech · 2026-09-02T08:05:06Z

+  checkmate::assert_numeric(x)
+  checkmate::assert_integerish(digits, lower = 0, len = 1, any.missing = FALSE)
+  checkmate::assert_number(zero_threshold, lower = 0, upper = 10^(-digits))
+  checkmate::assert_logical(whole_integer, len = 1, any.missing = FALSE)


checkmate::assert_flag()

wwojciech · 2026-09-02T08:06:17Z

+  round_type <- match.arg(round_type)
+
+  checkmate::assert_numeric(x)
+  checkmate::assert_integerish(digits, lower = 0, len = 1, any.missing = FALSE)


checkmate::assert_int()

wwojciech · 2026-09-02T08:12:51Z

 }
+
+#' @title Format numeric values by significant figures.
+#' @description `r lifecycle::badge('experimental')`\cr


Add 1 line space here.

wwojciech · 2026-09-02T08:13:47Z

+#' whole_integer and trailing zeros.\cr
+#' The underlying function for significant figures is `modified_signif_j()`, which is a modified version for
+#' [base::signif()]
+#' @details `format_sigfig_j()`\cr A function factory that produces formatting functions to round values to a


why format_sigfig_j()\cr ?

This is not the way to distinguish between different functions in roxygen. Please see a_summary_subset.R how this can be achieved in a proper way.

wwojciech · 2026-09-02T08:23:44Z

+) {
+  checkmate::assert_integerish(sigfig)
+  format <- gsub("xx\\.|xx\\.x+", "xx", format)
+  checkmate::assert_choice(format, c("xx", "xx / xx", "(xx, xx)", "xx - xx", "xx (xx)", "xx, xx"))


format can also by a function?, so better not to fix choices here.
Just check if this is a function or a string.

iaugusty added 2 commits August 18, 2026 13:40

formatting function format_sigfig_j

0e55595

NEWS and pkgdown

bcbe866

iaugusty requested review from gmbecker, jkovach2 and munoztd0 August 18, 2026 14:05

iaugusty and others added 3 commits August 20, 2026 14:05

minor update after discussion with WW

996511a

fix typo

7fd21c6

fix: removed @PARAM round_type and moved the @return

d0d0a59

wwojciech requested changes Aug 22, 2026

View reviewed changes

munoztd0 approved these changes Aug 24, 2026

View reviewed changes

munoztd0 and others added 3 commits August 24, 2026 15:49

Merge branch 'dev' into 436-FR-function-format_sigfig_j

44d1874

rename internal function signif_j to modified_signif_j

4292eed

update format_sigfig_j example code

3c0d535

munoztd0 requested review from wwojciech and removed request for wwojciech August 26, 2026 09:50

wwojciech requested changes Aug 27, 2026

View reviewed changes

adress feedback WW

5cd24a0

wwojciech requested changes Aug 29, 2026

View reviewed changes

iaugusty added 2 commits August 31, 2026 09:06

address feedback WW

e629170

example code for internal function

ab1a3cb

wwojciech reviewed Sep 2, 2026

View reviewed changes

wwojciech requested changes Sep 2, 2026

View reviewed changes

		@@ -384,6 +390,8 @@
		modified_signif_j <- function(x, digits = 6, round_type = valid_round_type, whole_integer = FALSE, zero_threshold = 0) {
		stopifnot(length(digits) == 1, is.numeric(digits))

Uh oh!

Conversation

iaugusty commented Aug 18, 2026

Pull Request

Checks

Uh oh!

wwojciech left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wwojciech Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

munoztd0 commented Aug 24, 2026

Uh oh!

wwojciech left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wwojciech left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wwojciech Aug 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wwojciech Aug 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wwojciech Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wwojciech Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wwojciech Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wwojciech left a comment •

edited

Loading

wwojciech Aug 22, 2026 •

edited

Loading

wwojciech left a comment •

edited

Loading

wwojciech Aug 29, 2026 •

edited

Loading

wwojciech Aug 29, 2026 •

edited

Loading

wwojciech Sep 2, 2026 •

edited

Loading

wwojciech Sep 2, 2026 •

edited

Loading

wwojciech Sep 2, 2026 •

edited

Loading