8000 Visualise bundles by milanmlft · Pull Request #81 · SAFEHR-data/omopcat · GitHub
[go: up one dir, main page]
Skip to content

Visualise bundles #81

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 41 commits into from
Oct 4, 2024
Merged

Visualise bundles #81

merged 41 commits into from
Oct 4, 2024

Conversation

milanmlft
Copy link
Member
@milanmlft milanmlft commented Oct 2, 2024 •
edited
Loading
  • Update plots to work with multiple concepts
  • Selecting a bundle from the dropdown now also selects the rows for the corresponding concepts in the datatable, and visualises the concepts together
  • Add an action button to clear selected rows

Closes #70

milanmlft added 16 commits October 2, 2024 15:10
@milanmlft
Change colour for monthly counts bar plot
6485d29
@milanmlft
Allow selecting multiple concepts from datatable
497309a
@milanmlft
Use .data$ prefix for plotting variables
bf317c1
@milanmlft
Round mean_persons and mean_records_per_person in datatable
2ca429a
@milanmlft
Use correct filtering for multiple concepts
6550f90
@milanmlft
Support multiple concepts for numeric stats plot
dd468c7
@milanmlft
Support multiple concepts for categorical summary stats
c573d52
@milanmlft
Get rid of the wrapper and do concept type filtering in the plotting …
770fd0d 
…functions
@milanmlft
Use a tabbed card to show both numeric and categorical stat plots
0523588
@milanmlft
Put plot titles in the card headers
d6a009d
@milanmlft
Update tests
cbd62b5
@milanmlft
Let select_bundle module return concept IDs instead of table
12400df
@milanmlft
Fix tests
16833be
@milanmlft
Update selected row sin datatable when selecting a bundle
66cdf99
@milanmlft
Improve test coverage
9a47e3f
@milanmlft
Add action button to clear selected rows
b9dd82b
@milanmlft milanmlft requested a review from a team October 2, 2024 16:51
@codecov Codecov
Copy link
codecov bot commented Oct 2, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.76%. Comparing base (03a21c7) to head (62f20fc).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main      #81      +/-   ##
==========================================
+ Coverage   93.30%   93.76%   +0.45%     
==========================================
  Files          19       19              
  Lines         508      545      +37     
==========================================
+ Hits          474      511      +37     
  Misses         34       34

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@milanmlft milanmlft marked this pull request as draft October 2, 2024 16:58
@milanmlft milanmlft marked this pull request as ready for review October 3, 2024 11:30
@milanmlft
Don't use plotly for summary stat plots
1c51eef 
`ggplotly` doesn't support boxplots with pre-computed values
plotly/plotly.R#565
stefpiatek
stefpiatek approved these changes Oct 3, 2024
View reviewed changes
Copy link
Contributor
@stefpiatek stefpiatek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking nice, a couple of tweaks and then it'd be great to get some feedback

@BaptisteBR
Copy link
Contributor

Looks great! I'm not a big fan of the concepts datatable, the fact that "everything" is scrollable (the "clear concepts" button, the search bar, the header, and the pages navigator)

@milanmlft
Copy link
Member Author

Looks great! I'm not a big fan of the concepts datatable, the fact that "everything" is scrollable (the "clear concepts" button, the search bar, the header, and the pages navigator)

Thanks for the feedback @BaptisteBR! I moved the "clear rows" button into the card header, so it remains visible. The other UI elements will require a bit more work, so will do that in a separate PR 🚀

@milanmlft milanmlft enabled auto-merge (squash) October 4, 2024 11:33
@milanmlft milanmlft merged commit bde35fd into main Oct 4, 2024
5 checks passed
@milanmlft milanmlft deleted the milanmlft/70-visualise-bundles branch October 4, 2024 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stefpiatek stefpiatek stefpiatek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Visualise all concepts in a bundle in the dashboard
3 participants
@milanmlft @BaptisteBR @stefpiatek
0