Biogen-Inc / tidyCDISC

Demo the app here: https://bit.ly/tidyCDISC_app
https://biogen-inc.github.io/tidyCDISC/
GNU Affero General Public License v3.0
108 stars 38 forks source link

Next release: `v0.2.1` #172

Closed AARON-CLARK closed 1 year ago

AARON-CLARK commented 1 year ago

Take note of steps from this stage of Cutting a new release wiki:

codecov[bot] commented 1 year ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@38cb7da). Click here to learn what that means. The diff coverage is n/a.

:exclamation: Current head 1c99cde differs from pull request most recent head 9d2d4af. Consider uploading reports for the commit 9d2d4af to get more accurate results

@@            Coverage Diff            @@
##             master     #172   +/-   ##
=========================================
  Coverage          ?   19.94%           
=========================================
  Files             ?       50           
  Lines             ?     4607           
  Branches          ?        0           
=========================================
  Hits              ?      919           
  Misses            ?     3688           
  Partials          ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

tdwils commented 1 year ago

@AARON-CLARK I'm going through the checklist, and I'll post my comments when I finish. But I just want to point out one issue I have found when using study data (not CDISC). The variables filenames and file_names are both being used, but I think they should be the same

The Shiny app code

https://github.com/Biogen-Inc/tidyCDISC/blob/71f0514823b5032fed1124d2d5619973ac4201dc/R/mod_tableGen.R#L745-L756

produces this script,

image

which gives an error because of the variable name:

image

tdwils commented 1 year ago

There is the same issue with the variables study_dir and study_directory. The variable names should be the same.

image

tdwils commented 1 year ago

With study data, grouped by ARM, variable = TEMP, ATPT = All, Stat = CHG, Visits = All"

image

Warning: Error in <-: 'names' attribute [6] must be the same length as the vector [3]
  [No stack trace available]
tdwils commented 1 year ago

This may be for a future enhancement:

If you create a facetted plot, separated e.g. by ACTARM, and the values for ACTARM are very long, the title of each facetted plot extends beyond the plot and is unreadable. Need to wrap the titles of the facetted plots. I have a screenshot with an example.

The same thing happens with boxplots with long x labels: they overlap and are unreadable.

tdwils commented 1 year ago

On the Individual Explorer:

I was testing the guide button [ ? ] under the Visits tab.

The last popup window does not have a "Done" button like the other guides, so when you click "Next", it just leaves you with a highlighted box. You can then click on the page to make it go away.

tdwils commented 1 year ago

I updated all packages to the latest versions and went through the checklist again (well, most of it). Here are some things I found:

Item 1 (latest versions of all packages)

Used CDISC data to generate Table 18 (Overall summary of AEs):

Warning: Specifying the `id_cols` argument by position was deprecated in tidyr 1.3.0.
ℹ Please explicitly name `id_cols`, like `id_cols = AOCCFL`.

Item 2 (latest versions of all packages)

Used study data. Loaded ADSL dataset. Attempted to create “Table3 Accounting of subjects”.

image

Input to asJSON(keep_vec_names=TRUE) is a named vector. In a future version of jsonlite, this option will not be supported, and named vectors will be translated into arrays instead of objects. If you want JSON object output, please use a named list instead. See ?toJSON.

This also happens if you try to generate “Table 5: Demography” and filter by ACTARM.

Item 3 (latest versions of all packages)

Individual Explorer: The guide button [ ? ] (in the Filter by Patient Number box) does not work like the other guide buttons. You have to click on the solid color within the square in order for it to work. If you click on the question mark, then nothing happens.

Item 4 (latest versions of all packages)

A lot more messages are getting printed to the console when the app is run:

image

Item 5 (latest versions of all packages)

In the Population Explorer with CDISC data:

image

The graph generates successfully, but this error is in the Console:

Warning: Error in vectbl_as_col_location2: Can't extract column with `value_y`.
✖ Subscript `value_y` must be size 1, not 0.
  155: <Anonymous>
  154: signalCondition
  153: signal_abort
  152: cnd_signal
  151: <Anonymous>
  139: eval
  138: eval
  137: .transformer
  135: <Anonymous>
  111: plotly::ggplotly
  109: %||%
  106: modify_list
  105: config
  103: renderPlotly [C:/Users/twilson4/Documents/RProjects_laptop/tidyCDISC/R/mod_popExp.R#274]
  102: func
   99: shinyRenderWidget
   98: func
   85: renderFunc
   84: output$popExp_ui_1-plot_output
    3: runApp
    2: print.shiny.appobj
    1: <Anonymous>

Item 6 (latest versions of all packages)

CDISC data, Individual Explorer, Visits tab. Click on Milestones or Adverse Events checkbox:

Warning: Unknown columns: `ATM`
Warning in ggplot2::geom_vline(data = vline_dat, ggplot2::aes(xintercept = !!INPUT_visit_var,  :
  Ignoring unknown aesthetics: text
Warning: Unknown columns: `ATM
AARON-CLARK commented 1 year ago

Thanks for the review. I opened issues for all your findings. @tdwils, @borgmaan, & @Jeff-Thompson12, please feel free to start knocking them out as we have time. Let's set a tentative goal to correct the easy ones and diagnose the trickier ones by end of week. Can discuss more at our dev meetup tomorrow.