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 indentation when knit_print() stopping rule list #836

Closed
fenguoerbian opened this issue Jun 25, 2024 · 3 comments · Fixed by #837
Closed

Add indentation when knit_print() stopping rule list #836

fenguoerbian opened this issue Jun 25, 2024 · 3 comments · Fixed by #837
Labels
enhancement New feature or request

Comments

@fenguoerbian
Copy link
Contributor

fenguoerbian commented Jun 25, 2024

Summary

knit_print() is a convenient way of rendering reader-friendly output of crmPack objects in Rmd report. But I see some improvement in the way of knit_print.StoppingList().

Currently there is no indentation added to the rendered stopping rule list. So if there are nested stopping rule list, they will all be at the same level, such as what is shown in the vignette, or an example as follows:

tmp_stop <- crmPack::.DefaultDesign()@stopping
#> Registered S3 method overwritten by 'crmPack':
#>   method       from  
#>   print.gtable gtable
crmPack:::knit_print.StoppingAny(tmp_stop) |>
    cat()
#> If any of the following rules are `TRUE`:
#> 
#> -  If all of the following rules are `TRUE`:
#> 
#> -  ≥ 3 cohorts dosed: If 3 or more cohorts have been treated.
#> 
#> 
#> -  P(0.2 ≤ prob(DLE | NBD) ≤ 0.35) ≥ 0.5: If the probability of toxicity at the next best dose is in the range [0.20, 0.35] is at least 0.50.
#> 
#> 
#> 
#> 
#> -  ≥ 20 patients dosed: If 20 or more participants have been treated.

Created on 2024-06-25 with reprex v2.1.0

The rendered output of these rules are all at the same level.

If any of the following rules are TRUE:

  • If all of the following rules are TRUE:

  • ≥ 3 cohorts dosed: If 3 or more cohorts have been treated.

  • P(0.2 ≤ prob(DLE | NBD) ≤ 0.35) ≥ 0.5: If the probability of toxicity at the next best dose is in the range [0.20, 0.35] is at least 0.50.

  • ≥ 20 patients dosed: If 20 or more participants have been treated.

What's the problem this feature will solve?

I think some indentation at the beginning of these lists are necessary so the rendered output one can easily see which rules belongs to which list, such as

If any of the following rules are TRUE:

  • If ALL of the following rules are TRUE:

    • ≥ 3 cohorts dosed: If 3 or more cohorts have been treated.

    • P(0.2 ≤ prob(DLE | NBD) ≤ 0.35) ≥ 0.5: If the probability of toxicity at the next best dose is in the range [0.20, 0.35] is at least 0.50.

  • ≥ 20 patients dosed: If 20 or more participants have been treated.

Currently I'm using a customized knit_print() that pads additional spaces to the begining of the output accordingly to achieve this. But I believe this feature would improve the readability and is worth considering to add into the package. If you like the idea I can submit a PR.

This issue can be related to #803 and #774

@Puzzled-Face
Copy link
Collaborator

Thank you for making this suggestion. I agree it's necessary - it's been on my "to do" list for a while. I will review asap.

@Puzzled-Face
Copy link
Collaborator

Puzzled-Face commented Jun 25, 2024

A couple of "nice-to-haves":

  • Please ensure consistency of case between "any" (for (StoppingListAny) and "ALL" (for StoppingListAll). I don't mind which.
  • Instead of "ALL", regardless of number, could we have "both" if N = 2 and "all" if N >2? Similarly "either" and "any".
  • A StoppingList with only one rule in the list is unnecessary, but possible. To allow for that, make the options "this"/"both"/"all" and "this"/"either"/"any".

@fenguoerbian
Copy link
Contributor Author

Thank you for this quick reply.

  • The inconsistency between cases of "any" and "ALL" is due to my typo. I think it's good to keep using small cases which is currently used in the package's code.

  • The "this"/"both"/"all" and "this"/"either"/"any" are really nice-to-haves and I like this for better output. Currently knit_print.StoppingAny() and knit_print.StoppingAll() just calls knit_print().StoppingList() and passes the preamble parameter. So for this option, we could leave the preamble parameter in the function definition without any defaults and generate the default preamble string in the function call (maybe another helper function New helper function for knit_print methods #789 here). When preamble is supplied, we can just use the user supplied version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants