CRI-iAtlas / shiny-iatlas

An interactive web portal for exploring immuno-oncology data. This repo now superseded by iatlas-app
https://www.cri-iatlas.org
Apache License 2.0
47 stars 12 forks source link

Refactor panels organization to follow Shiny modules approach #1

Closed jaeddy closed 6 years ago

jaeddy commented 6 years ago

See slides here for more details: https://github.com/daattali/shiny-training-rstudioconf-2018/blob/master/slides/05-modules.pdf

Basically, instead of

iatlas-demo/
├── global.R
├── panels
│   ├── panel-server-cellcontent.R
│   ├── panel-server-clonaldiversity.R
│   ├── panel-server-corrheatmap.R
│   ├── panel-server-survivalcurve.R
│   ├── panel-ui-cellcontent.R
│   ├── panel-ui-clonaldiversity.R
│   ├── panel-ui-corrheatmap.R
│   └── panel-ui-survivalcurve.R
├── server.R
└── ui.R

...we'd want something more like this:

iatlas-demo/
├── global.R
├── modules
│   ├── cellcontentmodule.R
│   ├── clonaldiversitymodule.R
│   ├── corrheatmapmodule.R
│   └── survivalcurvemodule.R
├── server.R
└── ui.R

Each module script would include a UI and server function — e.g., cellcontent_module_UI() and cellcontent_module() in cellcontentmodule.R. Module scripts would then be sourced in global.R and functions used in ui.R and server.R

Gibbsdavidl commented 6 years ago

Thanks for the link to the training slides. I'm reviewing them now. Very helpful. Template for modules: https://gist.github.com/jcheng5/c09a2449c0a94c8f498c4a68a416bc3b

Gibbsdavidl commented 6 years ago

Would we make the modules to follow the figure taxonomy? We can reuse the modules and have multiple links (with different analysis aims and titles) to each one.

iatlas-demo/ ├── global.R ├── modules │ ├── Scatter plot module.R │ ├── Boxplot family module.R │ ├── Bargraphs module.R │ └── Kaplan-Meier module.R | |-------etc.R ├── server.R └── ui.R

jaeddy commented 6 years ago

Thanks, @Gibbsdavidl — the module template also looks handy.

I was thinking that we'd try to follow the figure taxonomy, but with the modules themselves more organized by "bioapplication" as @vthorsson describes it. Something along these lines:

iatlas-demo/
├── global.R
├── modules
│   ├── samplegroupsmodule.R
│   ├── featurecorrelationmodule.R
│   ├── geneexplorermodule.R
│   └── survivalmodule.R
├── widgets
│   ├── scatterplot.R
│   ├── boxplot.R
│   ├── barplot.R
│   ├── mosaicplot.R
│   └── kmplot.R
├── server.R
└── ui.R

Such that functions for creating different widgets can be used by multiple modules. Not sure if this is the best approach, but trying to minimize duplicated code.

jaeddy commented 6 years ago

Modifying the structure a bit, based on @vthorsson's question about loading data...

Instead of "widgets", we'd have a place where we store re-usable "functions" (I think I've also seen these named as "helpers" — either would be fine with me).

iatlas-demo/
├── global.R
├── modules
│   ├── samplegroupsmodule.R
│   ├── featurecorrelationmodule.R
│   ├── geneexplorermodule.R
│   └── survivalmodule.R
├── functions
│   ├── scatterplot.R
│   ├── boxplot.R
│   ├── barplot.R
│   ├── mosaicplot.R
│   ├── kmplot.R
│   ├── load_data.R
│   └── utils.R
├── server.R
└── ui.R
jaeddy commented 6 years ago

@andrewelamb: below is a more detailed plan for mapping our existing content into the proposed reorganization. For this pass, the focus should be more on setting up the overall file/folder structure, not on consolidating code (which we'll try to tackle with #2). In other words, it's fine for now if code is duplicated in multiple files, as long as we can get the overall app to work.

This can probably be a single branch/PR, but might require multiple commits for any debugging that comes up...

iatlas-demo/
├── global.R
├── modules
│   ├── cellcontentmodule.R
│   ├── immuneinterfacemodule.R
│   ├── featurecorrelationmodule.R
│   └── survivalmodule.R
├── functions
│   ├── heatmap.R
│   ├── boxplot.R
│   ├── barplot.R
│   ├── kmplot.R
│   ├── load_data.R
│   └── utils.R
├── server.R
└── ui.R

modules

functions

andrewelamb commented 6 years ago

The functions are still in globals for now, but can be removed at any point.

andrewelamb commented 6 years ago

This is finished