1) The ucec_tcga data is not the same as production -- comparison/clinical tab shows 48 clinical attributes in this PR but 51 in production and I can also see slight differences in the genomic data. Can you make the data the same? I want to be able to use production to compare the output of the alterations tab.
2) Changing some setting (switching sample- vs patient- level enrichments or selecting/de-selecting groups or changing include/exclude overlap) resets the alteration types to include all types. The previously selected alteration types should be remembered.
3) 'Select Alteration Types' is hiding some of the graph:

https://ec2-18-157-159-210.eu-central-1.compute.amazonaws.com/comparison/alterations?sessionId=5fd0e0bde4b0aa1ac643ed1d&unselectedGroups=%5B%22Endometrial%20Carcinoma%22%5D
4) Not sure if this is also a data issue or a change in functionality, but when I compare Mutations on prod to Alterations with only Mutations selected on this PR, the number of genes is different
There's also a different in the number of rows in the table for the mRNA tab, and some of the q-values are marginally different. I presume this is data discrepancies between prod & this PR but can you please confirm/update the data to match prod?
5) My memory (which may not be accurate) was that the 'Select Alteration Types' was going to be a pop-up menu thing that would close (like Charts in Study View) rather than a permanent left sidebar thing. I would prefer it as something that could be hidden away, as it's taking up space that I think is better used for the graphs.
6) I also worry that 'Alterations' isn't a great name for the tab - we're replacing tabs called 'Mutations' and 'CNA', which are very clear about what data they include, with this somewhat vague 'Alterations'. In addition, in results view, directly above the tab name is a group called 'Altered group' which refers to a totally different set of alterations. I'm not sure how to fix this, but I'm worried it's going to be confusing. Maybe we call this new tab 'Mutations/CNA' instead? Or maybe others @cBioPortal/product have better ideas?

@tmazor Thnx for your feedback.
I have used the study as downloaded from the Data Sets tab. May it be that these studies differ from datahub studies, or that the loading mechanism is different?
I will address points 2 and 3 in my next commits. Point 4 I will investigate and report in a comment in this thread.
On point 5 I would like to mention the side panel will be converted into the button that you refer to in the PR that implements the Annotation filter menu. At present, I implemented it so that the panel will still show when the data does not support annotation based filtering (and there is no Annotation filter menu). I am fully ok with making it a button in this PR as well. Just make the decision in @cBioPortal/product.
I'm not sure about the consistency of data between those various sources - might be a question for @ritikakundra ?
For point 5, if the plan is to eventually make this a button, then I think we should do it that way in this PR. I think it ends up being more confusing if we start one way and then change it to a button later.
Hi Tali,
We would not merge this and then after a week merge the other PR. The idea
is to do one after the other in one go. There would be no actual release
that will have the menu as it is in this PR.
--
T. +31(0)30 700 9713
On Fri, Dec 11, 2020 at 3:37 PM Tali Mazor notifications@github.com wrote:
I'm not sure about the consistency of data between those various sources -
might be a question for @ritikakundra https://github.com/ritikakundra ?For point 5, if the plan is to eventually make this a button, then I think
we should do it that way in this PR. I think it ends up being more
confusing if we start one way and then change it to a button later.—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
https://github.com/cBioPortal/cbioportal/issues/8107#issuecomment-743228504,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ACMGKNOQN2PBJLAEB6ZNZHTSUIVDJANCNFSM4UT7F3NQ
.
Hello,
I have knowledge of Javascript node js, react js, MongoDB, Expressjs I want to contribute to this project but I'm new to the open-source community so how can I start can you help me to figure out how should I start.
Agreed with @tmazor's points.
About 6, maybe we name it as "Mutations/Fusions/CNAs"?
For the mutation types, are they going to be grouped: https://github.com/cBioPortal/cbioportal/issues/7847#issuecomment-684984170?
@tmazor we did check the 2 studies against portal and DH and the only diff was file names which we recently implemented. If the data was downloaded from Ramen, then we are doing sanity sync up till the PR for automated updates is deployed (which should be soon).
1234 - All look good - thanks @pvannierop for providing the same data in a production-like environment to compare against.
5 - Thanks for the clarification around the deployment plan @Sjoerd-van-Hagen . I do still think it's worth changing to a button in this PR since it will allow for testing a layout closer to the final plan, and prevent layout issues like 3 (above).
6 - Mutations/Fusions/CNAs sounds ok to me. It's long, but it gets the point across which I think is most important.
7 - I agree with @jjgao - would be nice to have the mutation types grouped. But independent of that, we should put some thought into the order - right now it looks pretty random to me, which makes it hard to scan & find alteration types of interest. In the absence of groupings, I think I would propose alphabetical order (perhaps with fusions artificially at the top), but we could also consider some sort of impact-based order.

8 - Also, is this list of mutation types dynamic to the types of alterations represented in the selected cohort? Or, asked another way, if a study doesn't have any fusions, will fusions still appear in the list? Based on what I'm seeing, I suspect that the list is not dynamic and is probably tied to all possible alteration types across all studies in the portal, which makes the list quite confusing & potentially misleading. Can this list be specific to alteration types that are actually present in the selected study?
9 - I think we discussed this issue and I can't remember if we had any solution in mind but... as I'm using the Alterations tab, one thing that I find frustrating is that when a gene is significant, I have no information about what type of alterations that gene has - is it mutated? amplified? deleted? all 3? Can we address this somehow?
10 - Related to 9 - The "Add checked gene to query" function has been removed in this PR. This is an important feature, and currently the easiest way to answer the questions mentioned in 9.
Prod:

This PR:

Hi Tali,
Thanks for all the testing.
Regarding 5, this would mean extra work for us, which will be overridden in the follow up PR. You can have a look at it of course, here: https://ec2-18-157-159-210.eu-central-1.compute.amazonaws.com/. The caveat is that the recent updates that we made to the alterations tab PR are not yet in the study view PR. So to make rebasing and getting everything merged easier, we need to get this alterations tab PR merged, followed by the study view PR.
Regarding 7, this has been shown the the community a couple of times. @jjgao has worked with Oleguer to come up with the list. I am not sure if they maintained any particular order.
Point 8, has also been discussed in the community and we have indicated that we do not have the resources to change this.
The same holds for point 9.
For point 10, there seems to be an issue because I think that this was working before. We will look into it.
About 7, here are the proposed grouping and ordering (https://github.com/cBioPortal/cbioportal/issues/7847#issuecomment-684984170)
About 8, I think it should at least be dynamic on the highest level, ie Mutations, CNAs, and Fusions. Currently, we will not show "Copy Number" tab is there is no copy number data.
About 9, maybe add a quick link to OncoPrint tab by querying the gene? In the future, we could bring the OncoPrint into the page.
@Sjoerd-van-Hagen if you don't have enough bandwidth for 8 & 9, we can try to help. I think they are important to fix.
@jjgao This is the menu after rework

@jjgao @tali The problem with #8 is that for fusion data I cannot see whether a study has fusions as part of the mutation data. So, for now I make this button appear together with the mutation section whether there are fusions in the database or not.
@tmazor @jjgao The demo server runs the newest version of the alterations tab.
@jjgao @tmazor Also the name of the tab is now dynamic. So, when there is no CNA data, for instance, the CNAs part is not included.
8 - The menu looks great like this. In terms of fusions, unless @jjgao has any ideas, I guess that's the best we can do until we fully transition to the new SV model?
One minor thing @pvannierop - can you capitalize "Structural Variants" in the menu for consistency?
10 - The button has returned! Looks good.
Thanks, @pvannierop. It's very well done and a great improvement!
@pvannierop could you please create issue(s) for the remaining items mentioned above?
@tmazor I have corrected the struct var name. @jjgao I have created a new issue with the remaining improvements here.
@jjgao @tmazor I have added the remaining items in a new issue as requested.