Skip to content

Add chart drilldown to concept pages#2806

Merged
chrisknoll merged 3 commits intomasterfrom
add_chart_drilldown_to_concept_sets_pages
Jan 10, 2023
Merged

Add chart drilldown to concept pages#2806
chrisknoll merged 3 commits intomasterfrom
add_chart_drilldown_to_concept_sets_pages

Conversation

@anton-abushkevich
Copy link
Contributor

Resolves #2805

@chrisknoll
Copy link
Collaborator

From the UI perspective, let's remove the 'time series' functionality from the concept sets: the way the aggregation is done across concept sets (talking the average of p10 or summing the monthly prevalence) is incorrect math, and an alternative approach to presenting the data could be identified.

The drilldown reports from the concept view is fine and helpful: it saves navigating to data-sources and finding the concept in the right domain. This is helpful and can stay. I'd keep that function for 2.13, and think about how to deal with 'concept-set reporting' as a 2.14 feature.

@TitrS TitrS force-pushed the add_chart_drilldown_to_concept_sets_pages branch from fe7467a to 2359340 Compare January 9, 2023 13:36
@TitrS TitrS requested a review from chrisknoll January 9, 2023 13:42
@TitrS TitrS changed the title Add chart drilldown to concept sets pages Add chart drilldown to concept pages Jan 9, 2023
@TitrS
Copy link
Contributor

TitrS commented Jan 9, 2023

@chrisknoll Hi Chris! I deleted last commits and keep commits for the concept drilldown report. I renamed PR also.

@chrisknoll
Copy link
Collaborator

Thanks. For future reerence, be careful about force-pushing a branch, if you want to go back to a prior commit, just revert to the desired commit which will cause git to make a 'revert commit' that removes the changes. I was lucky to notice you forced pushed the branch because if I tried to merge your latest commits' back to my local branch, it would have been chaos.

@chrisknoll
Copy link
Collaborator

Ok, The Atlas update looks good, but I'd like to also remove the endpionts on the WebAPI side that has those improper calculations that aggregate statistics. Before merging the Atlas side, let's remove the code from webAPI. To do so, please revert commits (don't force push branches) and I'll take a look.

@TitrS
Copy link
Contributor

TitrS commented Jan 10, 2023

Hi @chrisknoll thanks for your comments. If we decide don't add the functionality for concept sets page, you just should reject PR on the WebApi side OHDSI/WebAPI#2178, because on the WebApi side all changes are related to concept sets page.

@chrisknoll chrisknoll merged commit 677e1b8 into master Jan 10, 2023
@delete-merged-branch delete-merged-branch bot deleted the add_chart_drilldown_to_concept_sets_pages branch January 10, 2023 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Achilles drilldown reports to Concept / Concept Set pages

3 participants