-
Notifications
You must be signed in to change notification settings - Fork 6
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
Provide variable in pool dataframe showing which visit the row belongs to #255
Comments
Am thinking that this could be acheived by converting the analysis function to return a dataframe instead of a list. User can then insert arbitrary variables into the dataframe which are retained and combined into a final dataset. In theory this is also acheivable by keeping the list structure and just allowing the user/analysis function to insert arbitrary meta variables into it... |
@gowerc I see the nested list with concatenated visit names was originally created in
You said you want to convert the return value of Regarding your alternative suggestion, could you specify what does it mean by Another idea is to modify the data structure of returned attribute |
From @nociale
|
@nociale & @pengguanya , I think this is roughly in line with my original thoughts for how to handle this. I'm not a fan of the naming Another comment, I'm wondering if it makes sense to just take these meta values from the first iteration only and accept that as the "true" values. It would save us having to do a load of consistency checking and error handling. Just push it to the user in the documentation to say only the first returned result is used. |
@nociale, A suggestion that @pengguanya had mentioned was getting rid of the inner-lists and use an instantiation function / class instead. I liked the idea but he also correctly mentioned that there is a risk that we would then be changing exposed UI. I am wondering if the package is still niche enough for us to get away with changing this UI or not ? For example the analysis function would return like:
Perhaps this is overkill though... |
Hi both! Good that we agree on the structure! Regarding the name, I was making an example without focusing too much on it. I like I think it would be nice to get rid of one level of lists, and I think the solution is also smart. How would you change the UI then? For example, would a user be able to set |
O yes, to be clear the only place this impacts end-users would be in the custom analysis functions. Excluding the custom analysis functions the only thing this impacts is our own internal I.e. we would be asking users in custom analysis functions to return a list of analysis_results (or perhaps just "results"?) rather than a list of lists with fixed names. |
It looks good to me, this solution would simplify the output structure. Giving the possibility of inserting a "name" to each parameter being estimated looks also good to have a nicer output table from the |
Potentially useful for reporting to enable grouping / categorisation by visit
The text was updated successfully, but these errors were encountered: