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

Pilot 2 shiny app: call for review #80

Closed
lengning opened this issue May 6, 2022 · 4 comments
Closed

Pilot 2 shiny app: call for review #80

lengning opened this issue May 6, 2022 · 4 comments

Comments

@lengning
Copy link
Collaborator

lengning commented May 6, 2022

Please review

  1. the cover letter: R consortium R submission pilot 2 - cover letter.pdf, google doc version

  2. the deployed shiny app

Please feel free to leave your high level feedback in this issue, and specific bug reports etc in the pilot2 repo

Thank you!

code/ADRG review will be conducted in ~June

@nanxstats
Copy link
Member

Thanks Ning. I added a few minor suggestions, attached here.
R.consortium.R.submission.pilot.2.-.cover.letter.pdf

@SHAESEN2
Copy link

SHAESEN2 commented May 9, 2022

Thank you for sharing. Nice work done by the team. I had a few comments.

General

  • Should we think about a way for the FDA to (re)view the code behind the tables ? Personally, I think shinymeta works well for this.
  • Titles for the tables and explanatory footnotes might be helpful in interpretation of the tables. The description on the first page is not sufficient and difficult to remember (eg imagine you have 500 outputs to generate and the reviewer needs to go back and forth to the first page.
  • On the first page, should we include more information about the R version and the packages? We should at least mention the study and its purpose (imagine the reviewer wants to compare an Interim analysis with a resubmission and has the two apps open at the same time).
  • With regards to navigation between the different modules, how will you organize the app if you have 20 demographic tables? Will they all get a subtab under "Demographic" or will each table gets his own tab?

Demographic table

  • Should we allow the FDA reviewer to change the analysis set (eg FASFL instead of ITTFL)?
  • Can we allow the reviewer to save the table eg in HTML/PDF/TXT/... so that they can view results side-by-side? (eg cross-reference table/graph for the safety population).

KM plot

  • You need to foresee more height/less width on the HTML page. We are working on a new release of visR to improve the distance between table and graph (release within the next 2 months)
  • Why do we have a filter for the KM table? The only two filters that would make sense are ADTTE.PARAM/PARAMCD, the population flags and subgroups. However, the app does not seem to allow subgroup analysis eg If I choose ADSL.SEX then the graph display does not change. One more interesting filter could be to request n.censor/n.event/n.risk for the risktable.

Primary table

  • Primary table: I would use bold for the visit labels to make the table easier to read
  • Footnotes are not showing. I am referring to the numbers in brakcets eg [1][2]. If those are the contrasts then I would add a footnote to make that clear.
  • It is not clear for which population this table is (N is not matching with other tables)

Efficacy table

  • I prefer footnotes rather than hoover text
  • p-value (not p-Value)

@SHAESEN2
Copy link

@lengning do you want me to put these as issues in the pilot 2 repo?

@lengning
Copy link
Collaborator Author

lengning commented May 19, 2022 via email

@lengning lengning closed this as completed Jan 5, 2024
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

No branches or pull requests

3 participants