Separate unprofiled samples into separate group in comparison tab

[adamabeshouse]

cBioPortal/cbioportal#8411

Link

[alisman]

this condition could use a comment. like, i don't understand why we use "isSampleProfiledInMultiple" why multiple?

[alisman]

also, seems like a very expensive operation, right? samples x genes x more . maybe no way around it.

[adamabeshouse]

isSampleProfiledInMultiple is a method that checks whether a sample is profiled in multiple profiles, in a way that's more efficient than checking a single profile multiple times. the logic here (I'll add a comment because it is really hard to read) is we want to only consider a sample unprofiled if it is unprofiled in every gene in every profile. yes, it is expensive - samples x genes x profiles. the worst case is when everything is profiled, otherwise the _.every and the _.some would both return early. I cant think of a better way to check it unfortunately, let me know if you can

[jjgao]

LGTM. Thanks!

[tmazor]

Looks good!

[adamabeshouse]

@tmazor @jjgao @inodb @alisman the reason for the screenshot failures is that this PR redefines what it means for a sample to be designated "unprofiled" in the results view filter. whereas before, a sample is considered unprofiled if it is unprofiled for any gene x profile, now it is unprofiled only if for every gene x profile. Just want to confirm that's desired.

[jjgao]

@adamabeshouse the new logic makes sense.

[tmazor]

@adamabeshouse you're referring to what gets filtered by the option selected below?

I hadn't initially considered that the change in this PR would change how that filter functions. I think there is a use case for that filter to work as it previously did. Take this query: https://www.cbioportal.org/results/oncoprint?cancer_study_list=msk_impact_2017&Z_SCORE_THRESHOLD=2.0&RPPA_SCORE_THRESHOLD=2.0&profileFilter=mutations%2Cfusion%2Ccna&case_set_id=msk_impact_2017_cnaseq&gene_list=ACVR1%2520EGFR&geneset_list=%20&tab_index=tab_visualize&Action=Submit&hide_unprofiled_samples=true
I think it's reasonable to want to look at just the subset of samples where ACVR1 is profiled (current/previous behavior of the filter option) vs not being able to filter out those samples where ACVR1 is not profiled (updated behavior).

[adamabeshouse]

@tmazor I feel like the definition should be unified across tabs, otherwise it could be confusing? Maybe we add another setting that determines how we define "unprofiled"? Maybe it can be a sub-section in the global settings menu, with a radio button that determines whether its "unprofiled for any" or "unprofiled for all"?

Something like:

[tmazor]

That's a great point @adamabeshouse , and given that both options could be useful in difference scenarios, I like the idea of having a choice. However, if that radio button is also meant to control the definition of the unprofiled group in group comparison, I don't think that will be clear since it's under a filter checkbox. We want that unprofiled group to exist regardless of the status of that filter checkbox. Maybe it just needs to be a separate section in the filter menu after Annotate/Filter?

[adamabeshouse]

@tmazor great point... hmmm... Maybe under a "Definitions" section?

[tmazor]

@adamabeshouse yes let's try a 'Definitions' section!

[jjgao]

Good discussion. I like the idea of having the two options in the filtering menu. Also agreed with Tali that we will need to unprofiled group (not profiled in all). But I am not sure how a Definition section in the filtering menu would help much as it's hidden.

I am wondering if it is sufficient to clarify in the group description? When hovering over, we show the group description.

[adamabeshouse]

After some discussion, the decision is:

• Filtering has the two options, as in the above screenshot
• Comparison group is always just unprofiled for everything
• Add a detailed description to the comparison group on mouseover to clarify it.

[adamabeshouse]

@tmazor @jjgao I have implemented what we settled on in our discussion, let me know how it looks

[tmazor]

@adamabeshouse the unaltered group seems to have disappeared?

(from the link in the original issue)

[tmazor]

@adamabeshouse also all samples are excluded with the selection below, which doesn't seem right

[adamabeshouse]

@tmazor ah, good catch - its a bug with multiple study queries because it was considering all of the profiles for each sample, when obviously a sample would never be profiled in a different study...

Just pushed the fix, let me know if it works.

Also worth noting/discussing: I adjusted the filter warning to correspond with the two settings (I couldnt think of a quick way to juxtapose these so I just did screenshots of screenshots, please ignore the jankiness):

[jjgao]

@adamabeshouse looks good to me. Thanks!