Conversation
|
✅ All contributors have signed the CLA |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
It looks like we get default columns with 70 less lines of code. Nice.
Why do most picks() on the app have values()? I understand why we want to specify a value for filtering on a teal_transform_filter() or why tm_g_distribution() some parameters use values() while others don't: those with values is to specify a given value as reference, response or for plot customization.
But on other modules, like tm_g_scatterplot() or tm_t_crosstable(), values() is used even for x and y where I don't expect specific values to be needed:
From the description of picks on insightsengineering/teal.transform#270 it is not apparent that we need to specify it for merging and keeping those columns.
Answer: The values() is added to be able to filter dynamically on each variable. It is also reset if the selection changes.
| tm_g_scatterplotmatrix( | ||
| label = "Scatterplot Matrix", | ||
| variables = adsl_extracted_multi | ||
| variables = list( |
There was a problem hiding this comment.
Here we have list(picks()) instead of directly picks(). What does this add?
There was a problem hiding this comment.
Here we have list(picks()) instead of directly picks(). What does this add?
scatterplotmatrix have unknown number of "arguments" (variables), having a list here allows to specify variables from multiple datasets.
those with values is to specify a given value as reference, response or for plot customization.
Yup, filter on an dimension-variable (x, y, color, size etc) is set by default in the modules. I think it is a nice default when app-user can filter axis after selecting a variable. On friday's meeting we discovered that picks-arguments which allow multiple-variables, it is wrong to set a values()
From the description of picks on insightsengineering/teal.transform#270 it is not apparent that we need to specify it for merging and keeping those columns
Yup, I should link merge_module in the picks documentation
Related issues in the milestone:
See related PRs:
Installation