-
Notifications
You must be signed in to change notification settings - Fork 0
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
Enhancement: Column Mapping Feature #140
base: main
Are you sure you want to change the base?
Conversation
@Gero1999 I switched to bslib tooltips because the shinyBS ones weren't working. Basically the same thing though. I've left the tooltip messages pretty vague for now, only the grouping and adosedur ones have more info. Lmk if you want any of them to change. I figure once we add labels from #100 then some of the tooltips won't be needed or we can adjust them as needed. Also not sure about grouping_variables as part of pkconc and pkdose? can also be adjusted later, after you've merged #138 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice @js3110! I agree, once #138 is merged can be clever to considered, is just about adding the grouping_varaibles to both formulas.
Some feedback regarding optional details that could be added:
- I would prefer that the input calls are static and created in the UI (i.e,
selectInput(...))
). Otherwise it is very likely to produce issues, I guess that this could be the reason of whyshinyBS
was not working for you. - For our users label ID would be preferred to be explanatory names rather than CDISC variables.
- Hovering can add information about the column expected format, i.e, class expected such as numeric or categorical.
- When the user press
Submit Mapping
I would expect something to happen. Either a message notification like "Data mapped" or a Tab displacement -
grouping variables
could be better called something likeother grouping variables
so the user do not get the wrong perception and thinks the rest are not used and puts them two times. it could also be good to update the choices to elude them to put the same column as inANALYTE
,DOSNO
...
@Gero1999 have updated. its a lot more bulky now but the ui is static. the label IDs will be added when we do #100 so I have left it how it was. Not entirely sure what else to put in the tooltips but feel free to make suggestions if you think it needs changing. For grouping variables I have added a clearer description so the user knows what it is for, and if the columns are already named correctly the user cannot select the other columns anyway. |
Good job! I just detected some small things. Let me know if I can help and you agree with them:
|
@Gero1999 Ok it should be ready now, as agreed I have left grouping_variables for nca to you for after #60 has been completed. And for the input labels, again this will be included in #100 . |
hey @js3110 as discussed in chat we will accelarate one of the issues into this one directly. Just add an option for the user to apart from choosing column names in the unit inputs also standard concentration variables. Here are some helpful instructions:
|
@m-kolomanski @Gotfrid Hi, could one of you please check the code on this and review when you get the chance. There is a linter check saying it has too much cyclomatic complexity, so suggestions on how to fix this would be appreciated. Thanks! |
Hi @js3110 , I will be able to do the review earliest tomorrow. Cyclomatic complexity issue means that you have too many if-else branches in a function. In case of Shiny it pretty much always means shiny modules are required to extract pieces of server. |
ok thanks! I will created a nested module today so its ready by tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Issue
Closes #112
Description
We would like to offer the user better possibilities to define the column names that correspond with each column used by the app in the data. This will increase substantially the data flexbility and will also provide the user a better understanding of the App functioning
Definition of Done
The requirements for the feature to be complete:
Provide select/selectize inputs for the user to define each of the important columns. Preferably if the layout is nice and with easy searchable options. One proposal is divided in these groups:
Group identifiers.
Subject ID
,Dose Number
,Additional grouping
Concentration variables.
Analyte
,Analyte value
Dose variables.
Drug
,Dose
,Administration route
Time variables.
Time from 1st dose
,Time from last dose
,Dose duration
Unit variables.
Concentration unit
,Time unit
Hovering displays information about the column requirements or details. For example for administration route (
administration preferably stating either "intravascular" or "extravascular"
)Provide inputs for the user to define the columns for unit measures (e.g, AVALU)
Formulas for PKNCAdata and PKNCAdose need to include
grouping_variables
How to test
Data Section -> Column mapping
Try using dataset with renamed columns to see that it still works.
Contributor checklist