Skip to content
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

teal.slice example apps testing #526

Closed
3 tasks done
donyunardi opened this issue Jan 16, 2024 · 15 comments
Closed
3 tasks done

teal.slice example apps testing #526

donyunardi opened this issue Jan 16, 2024 · 15 comments
Assignees
Labels

Comments

@donyunardi
Copy link
Contributor

donyunardi commented Jan 16, 2024

Summary

We'll conduct a manual test on two example apps where we focus on the teal.slice features.

Please do this on two apps (dev version):

Please play around with the filtering capability and log issue if there's any finding.

@donyunardi donyunardi changed the title teal.slice UI testing teal.slice example apps testing Jan 16, 2024
@donyunardi donyunardi mentioned this issue Jan 17, 2024
34 tasks
@m7pr m7pr self-assigned this Jan 23, 2024
@vedhav
Copy link
Contributor

vedhav commented Jan 25, 2024

Blocked by insightsengineering/teal.modules.clinical#918
Will re-deploy the apps once this is merged and teal.gallery apps start to work.

@vedhav vedhav added the Blocked label Jan 25, 2024
@vedhav
Copy link
Contributor

vedhav commented Jan 26, 2024

RNA-seq and exploratory and ready to test. The deployment of early-dev has errors which I will fix on Monday morning.

@vedhav
Copy link
Contributor

vedhav commented Jan 29, 2024

All these apps are re-deployed. There is a bug in longitudinal app. But I'm looking into it.

@vedhav vedhav removed the Blocked label Jan 29, 2024
@m7pr
Copy link
Contributor

m7pr commented Jan 29, 2024

thanks @vedhav

@m7pr
Copy link
Contributor

m7pr commented Jan 29, 2024

Checked early-dev with respect to filter panel functionalities

Checked

  • runs at all!
  • all tabs show filter panel
  • It is possible to hide and show the filter panel
  • You can hide and show Active Filter Summary, Active Filter Variables and Add Filter Variables
  • I was able to add a new filter for a new variable that is shared across multiple tabs (so filters in this app are not module specific)
  • counts for ADRSSWIM were decreased both for Observations and Cases where a filter for applied, influencing other datasets to be changed as well
  • selection of NO levels for a filter with character/factor variables properly resulted in 0 observations being shown
  • restore and rewind arrows works for changing filter state
  • the same as above for remove filter icon
  • you can remove one filter or all filters for a dataset
  • it is possible to have multiple filters set
  • DESELECT and SELECT ALL options forks for a character/factor with many levels
  • usage of continuous variable in a filter works as intended: tooltip is readable, label is visible, the column name is used, you can set limits manually in 2 cells or with dragging lines on a plot (rewind arrow also works, restore original state as well)
  • single counts in one filter gets reduced when another filter is set and values are limited as well
  • Global Filters information is shown
  • Snapshot Manager allows to: Add a Snapshot, Save to File, Upload a Snapshot, Revert to Original State
  • Downloaded Snapshot is properly represented in JSON structure
  • When a Snapshot is Uploaded, the filters manager modal gets closed automatically and the filter is applied
  • You can add or remove Interactive Filters in Active Filters Variables is you have snapshots created and uploaded, and also they are extended after each variable was used (so you can always quickly turn it on or off)
  • Plots and tables get updated when within a filter data is limited due to removal of a level in a character variable or due to the limitation of continuous variable

Issues

Swimlane Plot

Regarding the testing of things not related to teal.slice

  • @vedhav I have found such an issue with Swimlane Plot. Would you be able to fix the app or diagnose which package caused that?
image

Refresh of the results when it's not needed

  • @gogonzo the app view (plots, tables) get updated/refreshed if a new variable is added as a filter, even though it contains all levels by default at start. Do we need to update plots / tables if there is no change in the data (only a filter is added, but it is yet not used). Just to go Variable Browser module and Add Filters Variable that does not affect observation count in Active Filter Summary but the whole view gets refreshed.

@chlebowa
Copy link
Contributor

@m7pr
Copy link
Contributor

m7pr commented Jan 29, 2024

Thanks @chlebowa - I do recall we talked about it before, but thought it's somehow resolved by now

@vedhav
Copy link
Contributor

vedhav commented Jan 29, 2024

@m7pr Sure! I'll look into the Swimlane module issue.

@vedhav
Copy link
Contributor

vedhav commented Jan 30, 2024

Turns out the actual bug was not module-specific and it was just because the app specified incorrect join_key parent-child mapping causing the filter panel to have wrong data.
I made this change in the teal.gallery please look at the PR insightsengineering/teal.gallery#132
Also logged a feature request in teal.slice which we can handle later #540

vedhav added a commit to insightsengineering/teal.gallery that referenced this issue Jan 30, 2024
Fixes the error reported
[here](insightsengineering/teal.slice#526 (comment))

Changes:
1. Uses proper parent-key mapping in the `join_keys` so the filter panel
provides the valid data to the modules.
2. Standardise the library calls across the apps.

<img width="946" alt="300445426-5aeca3ac-8f1b-45c3-8693-2f218a7e5fb5"
src="https://github.com/insightsengineering/teal.gallery/assets/49812166/3d866e16-21b5-4ddf-be00-90bcb689e173">
@m7pr
Copy link
Contributor

m7pr commented Jan 30, 2024

RNA-seq testing:

Checks

  • Filters for MAE datatype works as intended in early-dev

Improvements

@vedhav is it possible to select hd2 experiment as a default used on Forestplot? The current default one hd1 has this message displayed

No assays eligible for this experiment, please make sure to add normalized assays

Questions

A question for @lcd2yyz and @gogonzo . On the Reporter prevewier view I know you were trying to use YAML-like display for selected variables. This way each variable and it's level is displayed in a new row. Do you think this is optimal for variables with more than 20 levels? Maybe for cases with multiple levels we could actually bind into one line. Check out how much scrolling is in this card
image

@kartikeyakirar kartikeyakirar self-assigned this Jan 30, 2024
@donyunardi
Copy link
Contributor Author

@m7pr
It doesn't look ideal but then again, haven't heard any user feedback around this topic.
Let's make an issue so we can tackle and decide on this after the release.

@kartikeyakirar
Copy link
Contributor

kartikeyakirar commented Jan 31, 2024

thanks @m7pr for checklist it was quite helpful.
Checked nest_exploratory with respect to filter panel functionalities and everything works well.

Checked

  • runs at all!

  • all tabs show filter panel

  • It is possible to hide and show the filter panel

  • You can hide and show Active Filter Summary, Active Filter Variables and Add Filter Variables

  • I was able to add a new filter for a new variable that is shared across multiple tabs (so filters in this app are not module specific)

  • selection of NO levels for a filter with character/factor variables properly resulted in 0 observations being shown

  • restore and rewind arrows works for changing filter state

  • the same as above for remove filter icon

  • you can remove one filter or all filters for a dataset

  • it is possible to have multiple filters set

  • DESELECT and SELECT ALL options forks for a character/factor with many levels

@m7pr
Copy link
Contributor

m7pr commented Jan 31, 2024

@donyunardi I think we are ready to close this one. Two things as a follow-up from this exercise.

I would recommend opening a question in teal.gallery for RNA-seq app and discuss:

is it possible to select hd2 experiment as a default used on Forestplot? The current default one hd1 has this message displayed

For teal.reporter, maybe just a discussion:

On the Reporter prevewier view I know you were trying to use YAML-like display for selected variables. This way each variable and it's level is displayed in a new row. Do you think this is optimal for variables with more than 20 levels? Maybe for cases with multiple levels we could actually bind into one line. Check out how much scrolling is in this card

@vedhav
Copy link
Contributor

vedhav commented Jan 31, 2024

@m7pr Right now it is not possible to send the default experiment selection. I've created a feature request in tmh where this can be implemented. Thanks for pointing it out.

@donyunardi
Copy link
Contributor Author

Issues opened.
Thanks team!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants