-
Notifications
You must be signed in to change notification settings - Fork 1
create Anchor Screen Dashboard #243
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
My only question is about whether we want to continue the precedent of defining an legacy portal endpoint for each table.
But perhaps we can discuss in TPP?
@@ -574,3 +575,45 @@ def get_column_types(): | |||
return cols | |||
|
|||
return DataTableData(get_column_types, get_data) | |||
|
|||
|
|||
def get_anchor_screen_metadata_table(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh wow. This is a lot more hassle then I was expecting you to have to go through.
I was imagining the react component could just fetch the data from breadbox. (I was actually just having a similar conversation with Jessica yesterday and advocated for bypassing the portal backend.)
I know this is historically how we've handled tables, but do you see advantages to keeping this pattern? It has always struck me as extra complicated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way WideTable works currently wants a static link to point to for the CSV download. I didn't want to mess with that because there's something nice about it being a proper resource. We have some code to transform JSON to CSV on the frontend but it's not particularly well-written. So I just stuck with the tried and truth method.
<p> | ||
Below is a table of the anchor screens which have data analyzed and | ||
loaded into the portal. The “volcano” links will show a volcano plot | ||
of the differential analysis produced by “Chronos compare.” The | ||
“scatter” links will show a scatter plot of the drug arm vs the | ||
control arm. | ||
</p> | ||
<p> | ||
<b>Note</b>: the screens shown in the scatter plot are from | ||
processing all screens together with Chronos, whereas the Chronos | ||
compare volcano plots processed only the anchor screens. As a | ||
result, the differential effect size in the volcano plot should be | ||
similar but not exactly the same as the difference shown in the | ||
scatter plot. | ||
</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it was set to a given_id and you tried to select a different feature/sample, it would get confused.
Asana:
https://app.asana.com/1/9513920295503/project/1165651979405609/task/1209952772841305