Skip to content

Add formatting functionality to package - #8

Open
Yeon-Choi-git wants to merge 6 commits into
build-r-packagefrom
add-formatting-functionality-to-package
Open

Add formatting functionality to package#8
Yeon-Choi-git wants to merge 6 commits into
build-r-packagefrom
add-formatting-functionality-to-package

Conversation

@Yeon-Choi-git

Copy link
Copy Markdown
Collaborator

Hi @CarlosPoses,

I made some minor updates to your mask_vector() so that:

  1. it can take an input that is a dataset in a wide format (like one row data set instead of a vector)
  2. it always return a character string consistently, even when there is no number that were masked.
    • add an option to add a big number separator
    • show trailing zeros until the intended decimals

Please see an example.

testdat <- data.table::data.table(var1 = 4, var2= 2, var3 = 4000)

testdat #data.table object
  var1  var2  var3
 <num> <num> <num>
     4     2  4000

mask_vector(testdat, 
            threshold = 5, 
            rounding_digits = 2,
            big_mark_seperator = ",",
            output_warnings = TRUE, 
            percentage = FALSE,
            total = NULL)
         var1            var2            var3 
      "[1-4]"         "[1-4]" "[3,998-4,004]" 

mask_vector(testdat, 
            threshold = 5, 
            rounding_digits = 2,
            big_mark_seperator = ",",
            output_warnings = TRUE, 
            percentage = TRUE,
            total = NULL)
         var1            var2            var3 
"[0.02-0.10]"   "[0.02-0.10]" "[99.80-99.95]" 

@Yeon-Choi-git Yeon-Choi-git self-assigned this Oct 14, 2025
@Yeon-Choi-git
Yeon-Choi-git changed the base branch from main to build-r-package October 14, 2025 11:27
@CarlosPoses

CarlosPoses commented Oct 14, 2025

Copy link
Copy Markdown
Collaborator

Hi Yeon,

Thanks a lot! Nice features.

  • For 2 (formatting), I will merge without changes.

  • For 1 (accepting dataframe as input), I would suggest adding a small wrapper to support this instead of allowing a dataframe or datatable object as input for the mask_vector() function.

    My rationale: as the package is now, there're two masking functions.

    1. Mask vector(): takes vector as input and masks that vector. I think it's good to keep this for vector input only.
    2. Mask_vector_wrapper(). Takes DescriptiveTable output (which is just a particular dataframe structure created with this package), identifies which vectors need to be masked from this table and applies the masking.

    More generally depending on the data.frame or data.table structure you want to use the function with, which rows and/or columns combinations correspond to the vector that need to be masked will be different. It totally makes sense to add your specific case, but I would add one different wrapper for each different data.frame or data.table. For instance I would call this wrapper mask_vector_row_tables(), having a third function.

    Would you agree? It's a small thing but I think it would help in maintaining or scalating. For instance if someone else wants to use this function for a different data.frame structure, it's clear they need to write a new wrapper that specifies what are vectors to be masked. I am happy to write the code. Might as well through an error for non-vector inputs and add some documentation about this wrappers idea.

Carlos

@Yeon-Choi-git

Copy link
Copy Markdown
Collaborator Author

Yup make sense wil do that. thank you!

also add a condition to return formatting outputs when there is nothing to mask
Comment thread R/masking-functions.R

# IF nothing to be masked, return input_vector with formatting.
# This ensure vectors that need a masking procedure will be in a string format.
if(all(input_vector >= threshold | input_vector < 1)){

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.

Added a condition to also return formatting values when a vector contain 0 value to mask.

Comment thread R/masking-functions.R
Comment thread R/masking-functions.R
@Yeon-Choi-git

Copy link
Copy Markdown
Collaborator Author

hi @CarlosPoses,

I made a new push including:

  1. added a condition in masking_vector() to return formatted numbers also when the input vector contains no values to mask.
> # Nothing to mask
> mask_vector(c(0,5,1000),
+             threshold = 5,
+             rounding_digits = 2,
+             big_mark_seperator = ",",
+             output_warnings = TRUE,
+             percentage = FALSE,
+             total = NULL
+ )
[1] "0"     "5"     "1,000"
  1. a wrapper function as you suggested - which can also perform the secondary masking per row.
> testdat <- data.table::data.table(var1 = c(1, 4), 
+                                   var2 = c(0, 2), 
+                                   var3 = c(10, 3), 
+                                   var4 = c(3000, 4000)
+ )
> testdat
    var1  var2  var3  var4
   <num> <num> <num> <num>
1:     1     0    10  3000
2:     4     2     3  4000
> mask_row_table(input_row_table = testdat ,
+                 maskcols = c("var2", "var3", "var4"),
+                 threshold = 5,
+                 rounding_digits = 2,
+                 big_mark_seperator = ",",
+                 output_warnings = FALSE,
+                 percentage = FALSE,
+                 total = NULL)
    var1   var2   var3          var4
   <num> <char> <char>        <char>
1:     1      0     10         3,000
2:     4  [1-4]  [1-4] [3,997-4,003]

@LindaNab

Copy link
Copy Markdown

@CarlosPoses can you look at the above please? Preferably we would merge this into main

Comment thread R/masking-functions.R
## If the input is an exception left as is
if(length(input_vector) == 1 & is_exception(input_vector[1])) return(input_vector)
if(length(input_vector) == 1 & is_exception(input_vector[1])) {
return(formatC(input_vector, big.mark = big_mark_seperator))

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.

This crashes for some values of is_expection = TRUE. Concretely formatC seems to not support NA and NaNs

expected value of mask_vector is character now, so reflected in tests
@CarlosPoses

Copy link
Copy Markdown
Collaborator

Hi @Yeon-Choi-git ,

I just look at it, sorry for the long waiting time. The function crashes when running the tests (you can source it tests/testthat/test-interval-masking.R' and it's related to the new formatting.

  1. Is the expected output of mask_vector always a character now? This seems fine to me. I assumed so, so that's what a test that was failing reflects now.
  2. The error seems that the new formatting functionality doesn't handle well the exception values. Concretely the problem is caused by NA and NaN values now being an acceptable input to formatC. Could you take a look?

Thanks!

Carlos

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.

3 participants