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

Issue with simultaneously grouping and filtering #218

Open Jeff-Thompson12 opened 1 year ago

Jeff-Thompson12 commented 1 year ago

(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.

Jeff-Thompson12 commented 1 year ago

Reopened this issue because I realized that the second item is related to the code base and not error handling.

The totals data frame used to create the block only has one treatment after the filtering but the app method still wants the other treatments/levels.

It is unclear to me what the desired behavior would be. When do we want to drop grouping levels? And when should we have a column of 0's?

AARON-CLARK commented 1 year ago

The totals data frame used to create the block only has one treatment after the filtering but the app method still wants the other treatments/levels.

It is unclear to me what the desired behavior would be. When do we want to drop grouping levels? And when should we have a column of 0's?

That is a good question. At first, I would think we'd want to retain the grouping var levels that existed before IDEAFilter punted them, creating columns of 0's in the output... but then again, some population filters get rid of unnecessary treatment levels so folks would want those removed from the output when filtering. Ideally, we could make the users decide if their filtering should impact the denominator or not, but that sounds like it could get tricky, no? Perhaps we address this after our v0.2.1 CRAN release?