-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add RNA types #170
Add RNA types #170
Conversation
oops, I didn't realise this was a pull request not an issue, and I made the same change also.
hmmm, i don't like it. Its breaking the separation between the data and the representation of the data. Also, I think it makes complex viewer less easily reusuable as a component in different contexts.
neither of these things is really a problem, it was easy for you to add things to that list ( ComplexViewer/src/js/read-mijson.js Lines 236 to 296 in 45c9044
and I also don't mind maintaining it.
its getting the thumbs down from me :) The two real problems with the MIJson are #160 Re. #155 - the unknown interactor types reffered to in the title were easy to resolve, just like here. But the issue at the end is a bit tricky, seems like the json can't really record the situation, like @bmeldal sayas hacking the viewer prob isn't the right thing to do. |
I agree with the argument about the separation and making integration of the viewer by a third party easy. The problem is that we never know that an interactor type is missing until we accidentally find a case that's broken. So a way that doesn't rely on hard-coding type and symbol but using the MI ontology to find the right branch for the type would be ideal - not? In most cases the missing symbols were "new" types of RNAs (new = not use as an interactor yet, not new to biology) so being able to identify them as "RNA" and then grabbing the right symbol would be best. (I'm just a dumb curator, so what do it know ;-) ) |
Believe it or not @bmeldal but that's exactly what I'm doing on the backend! (so of course you're not dumb at all ;) ) And basically, we needed it for both IntAct App, and IntAct Portal, hence the centralisation in the backend. I totally understand your concern @colin-combe as I had exactly the same before, it just happens to be simpler, in the end, to implement styling once and for all in the backend than on every front-end of ours, especially when we need to update the styles ^^ So I won't insist anymore on that personally, just know that if at some point you want to use it, it can be there ^^ |
By the way @colin-combe |
We added a new type for small molecules as well that you are missing on your side: |
yes, i'd agree with that as a short term thing i'll add those two other types also, but I agree this can be improved... |
ok, they're added (v2.1.6), I wonder if there are other ones missing... I should check it properly @bmeldal - I'll make a GH issue along lines you suggest the biggest prob with the hardcoding (perhaps) is that it means updating it in CP, Intact and the Editor every time it changes:- |
I think those are 2 different things. Hard-coding means you have to make the change each time we use a new interactor type and curators don't know when this happens as they are often already in the DB. If a curator asks for a completely new CV term we can ask whoever is taking over the PSI-MI CV to let you, Colin, know and you can proactively add the new links at the time. I'm not sure we can do much about the fact that we need to redeploy each applications in order to update to the next viewer version. However, we have other 3rd party widget in the CP (Reactome, LiteMol, GXA). @EliotRagueneau can you see how these would be updated? |
yes, I agree it's not a good system. #173 would fix this? |
We were having an error regarding this interaction so we updated the different types.
I created a new service in our backend to determine the style of our different representations and we were thinking to use that to send directly the shape that the elements should have inside the MIJson. This way we wouldn't have those problems of missing types, and you wouldn't have to manually check the types of interactions. What do you think about it @colin-combe ? Should we work on that, or is it overkill?