m2ms / fragalysis-frontend

The React, Redux frontend built by webpack
Other
1 stars 1 forks source link

Fix behaviour of RHS [P] button - also RHS upload change #1249

Open matteoferla opened 8 months ago

matteoferla commented 8 months ago

We need to allow users to display, alongside RHS compounds, the most appropriate protein. This could either be:

  1. an uploaded conformation - that's covered in #1222 spec
  2. the most appropriate LHS protein - scope of this ticket.

So need; A. an extra field in the RHS uploads - lhs_pdb. One of lhs_pdb or ref_pdb needs to be populated. B. (in this ticket) [P] should activate the LHS pdb pointed to by lhs_pdb in the SDF.

Furthermore:

(https://github.com/m2ms/fragalysis-frontend/issues/1222 has additional F/E spec of what to do once there are two relevant proteins.)

(@mwinokan's previous spec.)

As discussed in Slack, it may be beneficial for the RHS compounds to autoload the first compound and the side chain of its chosen template.

The reason for this is when RHS compounds are reviewed by someone that does not know there may be a difference they may disregard all the compounds erroneously due to clashes, whereras if sidechains were shown automatically it would be fixed.

This would probably need the same safeguards to not cause the same issue as https://github.com/m2ms/fragalysis-frontend/issues/1242

kaliif commented 8 months ago

So need; A. an extra field in the RHS uploads - lhs_pdb. One of lhs_pdb or ref_pdb needs to be populated. B. (in this ticket) [P] should activate the LHS pdb pointed to by lhs_pdb in the SDF.

It seems that pressing P fetches the PDB file from the backend just by name. As the PDBs are consistently named, it looks like the problem is simply figuring out which file to get. This seems to be just frontend issue or am I missing something?

phraenquex commented 8 months ago

These three buttons should use whatever is pointed to by lhs_pdb

image

phraenquex commented 8 months ago

Should point to the SiteObservation, not just some PDB. Just like on the LHS. And so makes it consistent with behaviour in Insprirations modal.

image

phraenquex commented 8 months ago

Also: @alanbchristie has to implement the lhs_pdb field.

matej-vavrek commented 8 months ago

RHS dataset molecule looks for site_observation_code property to link its P, C, S buttons with corresponding site observation molecule.

phraenquex commented 8 months ago

Looks great.

Remaining bug: protein colouring now has unexpected behaviour: the protein has the colour of the RHS compound (correct); and the corresponding LHS [P] is activated (also correct); but toggling the LHS [P] off then on brings back the protein still in the RHS colour. Required:

For proteins where there are remembered settings, this is what should happen: use the adjusted/remembered settings, but if turned on from RHS, apply the colour of the corresponding RHS compound.

phraenquex commented 8 months ago

@alanbchristie @kaliif does this still need backend work? Please confirm.

alanbchristie commented 8 months ago

I believe the b/e now provides information found in the upload, specifically the ComputedMolecule object now has a site_observation_code field/property, which should be the LHS SiteObservation for the molecule. So (I think) the b/e is complete.

phraenquex commented 8 months ago

Um. Top of this ticket, there was an item "A":

So need; A. an extra field in the RHS uploads - lhs_pdb. One of lhs_pdb or ref_pdb needs to be populated. B. (in this ticket) [P] should activate the LHS pdb pointed to by lhs_pdb in the SDF.

So my question was, has A been implemented. The thing you describe does not appear to be equivalent to the spec, though I didn't compoletely follow the meaning.

@mwinokan can you help @alanbchristie figure this out?

mwinokan commented 8 months ago

@alanbchristie please confirm to me that the current behaviour is as follows:

So what is missing is a way to specify both the computed and LHS reference in the uploaded SDF. Matteo's spec (below) for this is still relevant.

A. an extra field in the RHS uploads - lhs_pdb. One of lhs_pdb or ref_pdb needs to be populated. B. (in this ticket) [P] should activate the LHS pdb pointed to by lhs_pdb in the SDF.

alanbchristie commented 8 months ago

According to the implementation, the ComputedMolecule inspirations are (should be) the SiteObservations identified by the ref_mols property.

For reference this is done in the set_mol() function in cset_upload.py and the resultant inspirations array/list stored in the ComputedMolecule object's computed_inspirations field/column.

The ref_pdb is used to populate the ComputedMolecule site_observation_code.

For reference this is also done in the set_mol() function in cset_upload.py.

It looks like the lhs_pdb is not used, but ref_pdb is expected.

mwinokan commented 8 months ago

Thanks @alanbchristie

The ref_mols mapping to inspirations (SiteObservations) seems correct to me.

But there is still a need for the ComputedMolecule's to have both a LHS reference protein and a computed reference protein. I suggest that lhs_pdb should be used to populate the ComputedMolecule's site_observation_code and ref_pdb should populate another field which specifies the name of the computed reference PDB. An error should be throw if neither ref_pdb or lhs_pdb are present for a given row in the SDF, but both are not required.

alanbchristie commented 8 months ago

OK - I will make this modification later (I'm offline this afternoon).

mwinokan commented 8 months ago

There is another colouring bug: Activating a [L] view on a RHS compound and then using the arrows to navigate to adjacent compounds correctly changes the ligand being rendered, but the colour is incorrect.

See below (correct):

Screenshot 2024-01-11 at 10 50 50

See below (incorrect) the ligand should be green and not pink:

Screenshot 2024-01-11 at 10 50 59
mwinokan commented 8 months ago

On the LHS there are inconsistencies between the colour of the 2D drawing and the NGL view

Screenshot 2024-01-11 at 10 58 31
alanbchristie commented 8 months ago

Can someone provide a 'real world' example of a lhs_pdb value (as I do not have one)? I have ref_pdb and I assume the lhs_pdb will be of the same syntax, i.e. will look just like a ref_pdb value?

>  <ref_pdb>  (3) 
x0051_0B
mwinokan commented 8 months ago

@alanbchristie the example you have x0051_0B is a reference to a LHS protein structure and so you can use that for as lhs_pdb example data. The ref_pdb syntax will need to be appropriate to point to a RHS protein structure file

Waztom commented 8 months ago

@alanbchristie and @mwinokan will check in with frontend to confirm ref_pdb and lhs_pdb behaviour.

mwinokan commented 7 months ago

Thanks @matej-vavrek the colouring looks correct on your stack.