search

insightsengineering / teal.modules.hermes

RNA-seq analysis modules to add to a teal application
https://insightsengineering.github.io/teal.modules.hermes/
Other
7 stars 1 forks source link

bug fix #349

Closed chlebowa closed 11 months ago

chlebowa commented 11 months ago

Fixes this issue

Modified example for adtteSpec to also include ADSL dataset. :exclamation: I had to take it from teal.data so it's probably not the ideal solution. Adapted experimentSpec to new teal_data class.

vedhav commented 10 months ago

Hey @chlebowa since the teal_data passed to the UI is being depreciated, what are your thoughts on using reactive(<teal_data>) throughout the places regardless if it's a module UI or module Server?

I kinda liked the convention that the UI functions use a non-reactive teal_data and will not try to rerender when the teal_data changes. Because of this change, we have to pass the reactive data https://github.com/insightsengineering/teal.modules.hermes/pull/348/commits/d4c1b9882a10555ae923a0d0bcdde23ba0f46db6 and I think this is not right because the filter state change should not affect the encoding panel UI and passing a non-reactive data communicates this. Whatever we decide, we will have to do the same in the teal.modules.helios's experimentSpecInput too.

chlebowa commented 10 months ago

Hey @chlebowa since the teal_data passed to the UI is being depreciated, what are your thoughts on using reactive(<teal_data>) throughout the places regardless if it's a module UI or module Server?

I don't understand the question. We are moving away from using data in modules' ui functions.

Encodings depending on data (and therefore invalidating when filter state changes) would be an issue, yes. There are plans to deal with it.