-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Change constructor of the FilteredData to not depend on TealData #33
Comments
In this post when I say FilteredData - I mean the object which contains the data used within the shiny app - at the moment this is the FilteredData object itself but in the future can be the data object which is passed into modules.
If we specify data as a named list then we also need to specify: metadata, label and (primary) keys to FilteredDataset and then join_keys and (for cdisc) parent to FilteredData - as discussed with @kpagacz and @gogonzo we're not having CDISCFilteredDataset...
and
2 )But this is a little strange as parent, metadata belong with each dataset so you could do it this way:
2+) But then you might as well wrap each one up into a TealDataset object and then you can specify FilteredData in exactly the same way as you specify a TealDataset in teal_data - except you cannot have connectors only TealDatasets. (Maybe you can simplify TealDataset if you just pass in a raw dataset and don't care about reproducility etc.) you also get the structure and validation as well. This then keeps a link between teal.slice and teal.data though you could pass a TealDataset directly into a module
And this schema could be read from say chevron's yaml file...but presumably you would want to use the same code to specify a teal_data object as well. I guess a nice thing about doing it this way is that modules won't nec need the schema part (especially if no merge or merge happens before) and could just take the data - but maybe we don't want that actually. For this issue I suspect we'll do 1) as it's easier and can be changed in the future but something feels a bit strange here - having two different ways of specifying the same thing - 1) if you want to use teal::init and teal_data to specify your data/schema/metadata and a different way of specifying it if you want to use the data by itself (even if you need to specify the same things). |
@nikolas-burkoff I like (2) as dataset attributes goes with dataset. I'd prefer the object+attributes instead of list to be consistent with this. Not having CDISCFilteredDataset is a good thing, let's stick to have FilteredDatasets dispatched by raw object. In a long term we expect FIlteredData to be managed by the API. We will have a FIlterStates object which holds all current states and filter_panel_srv/filter_panel_ui which renders and manages filterstates. FilterStates can be also changed in other places. |
part of insightsengineering/NEST-roadmap#32
CodeClass should not be included in the FilteredData. But it's "silently" used in get_rcode to combine reproducible code. We need to find the way how to assure reproducibility in the teal_module(s)(to be addressed later)So by default it should be possible to initialize FilterPanel using following:
Things to resolve:
The text was updated successfully, but these errors were encountered: