Open EliotRagueneau opened 5 months ago
@colin-combe Was that intentional, or is it a blind spot ?
actually, i thought that worked as you would expect and it handled this correctly. I'll look into whats going on and get back to you (next week probably)
Thanks a lot Colin !
We noticed that sometimes it works, like on the complex https://www.ebi.ac.uk/complexportal/complex/CPX-5684, but that is because for some reason, the participant is duplicated, rather than having a stoichiometry of 2.
thanks, maybe that's why i thought it worked, or maybe i assumed it would always get the data like this
Hi guys,
still loitering here :)
It's true, the subcomplex stoichiometry doesn't show in the viz. Not sure why but both examples are my "mine" ;-) And I can tell you the difference: We know the specific subunit linked features for all components in the SARS complex but we don't know the specific subunit links for the polymerase and therefore only list 3 participants (all complexes in itself) with stoichiometry.
Over to you for figuring out why you don't extrapolate the stoichiometry for the unlinked complex.
Btw, nice addition to expand the subcomplex in the supercomplex view, much more human-friendly :)
Birgit
Hi @bmeldal !
Thanks for the explanations, that makes sense. We just need to make it work with stoichiometry too now I guess ^^
If you're speaking about the legend being clickable, yes, that's a nice little touc introduced by @jmedinaebi 😄 In the viewer itself, I think thta feautre was there for quite a while no?
Yes, I was referring to the legend being expandable. Before, you had to open the subcomplex in a new page to see the participants.
@EliotRagueneau - doing archaelogy on my own code... it looks like i started trying to fix this but never finished. Looks like i was thinking about this when i added the code here https://github.com/MICommunity/ComplexViewer/blob/master/src/js/clone-complex-interactors.js
However the commit is called "towards fixing multiple instances of same complex", implying it never arrived at the fix.
Also, it clearly can't work because it doesn't reference the stoichiometry (it only mentions it in an incorrect comment). Will continue with it... i find that code to do with stoich expansion isn't super easy to figure out.
@bmeldal - hi Birgit
Hi @colin-combe :)
that code in https://github.com/MICommunity/ComplexViewer/blob/master/src/js/clone-complex-interactors.js is actually doing something else...
you can see this apparently working now at https://complexviewer.org/
The code is quite confusing though. There's really three steps in the 'expansion' process:
For CPX-5684 & CPX-5605, the situation is a bit difficult to describe - the complexes that appear twice, appear twice as participants with stoichiometry one; but they aren't actually duplicated in the JSON, rather there is two separate references to the same complex (which is described once in the JSON). It means we still need to clone them and their participants to make them appear twice in CompexViewer (step 2 above). I guess what it's doing with these is giving the correct results?
Though this looks like it works for the current examples, i see something strange in the code for step 2 and i think it might not work if there were more than two references to same complex. So, I'm thinking to test / look at this a bit more.
Does anyone know of an example like CPX-5684/CPX-5605 (multiple references to same complex, each with stoichiometry one) where there are more than two references to the complex? Or an example that mixes the two approaches (complex with stoichiometry > 1 and complex with multiple references)?
I checked in the database and the only complexes having 2 participants which are actually the same, like for CPX-5684, are the following:
All of those examples are only involving 2 instances of the sub complex, it looks like there are no complexes more duplicated components like this.
CPX-5602 doesn't seem to fit in that category, and the rendering looks correct to me. Regarding subcomplexes with a stoichiometry, it seems that CPX-1924 is the only one concerned. You could mock it though by modifying locally a json file to combine those 2 aspects and see if it would still respond as we expect.
You could mock it though by modifying locally a json file to combine those 2 aspects and see if it would still respond as we expect.
yes, I'll do that. I'll get back to you before end of week.
viewer fails on https://www.ebi.ac.uk/complexportal/complex/CPX-7083
CPX-5602 doesn't seem to fit in that category, and the rendering looks correct to me
yes, the whole time is was saying 'CPX-5602' above, i meant CPX-5605. Which does fall into that category i think.
CPX-7083 is failing because of the so called 'step 2' above
CPX-5602 doesn't seem to fit in that category, and the rendering looks correct to me
yes, the whole time is was saying 'CPX-5602' above, i meant CPX-5605. Which does fall into that category i think.
This one is a bit more tricky as the 2 similar complexes are not at the same level of nesting, but I think the rendering is eactly what we expect there, unless maybe that they could share the same background
this specific issue (not expanding complex stoich in CPX-1924) should be fixed by https://github.com/MICommunity/ComplexViewer/pull/201
CPX-7083 is still broken, will make seperate issue for that
the rendering is exactly what we expect there, unless maybe that they could share the same background
i think the share same background colour suggestion is good, that makes more sense (and works better for your legend). That's changed by https://github.com/MICommunity/ComplexViewer/pull/201 also
Awesome! Thanks a lot!
you can see the shared colours at complexviewer.org
e.g. in https://www.ebi.ac.uk/complexportal/complex/CPX-1924, we have subcomplexes with a stoichiometry, but that stoichiometry is not reflected in the visualisation. Probably need to work on https://github.com/MICommunity/ComplexViewer/blob/3fda5025e2afc0133a8b1c137c4d5c440641b756/src/js/expand.js#L28-L65