PistoiaHELM / HELMWebEditor

Web browser based editor for drawing HELM macromolecules.
MIT License
18 stars 10 forks source link

Ambiguous bond information is only partially displayed #148

Closed ClairePA closed 6 years ago

ClairePA commented 6 years ago

If an ambiguous bond between groups has only some of the attributes defined, only the defined attributes are displayed. This is confusing.

image

All attributes should be shown.

scilligence commented 6 years ago

Could you paste the HELM notation here? I guess the missing information is not specified. Thanks.

ClairePA commented 6 years ago

The HELM is:

PEPTIDE1{A.C.D.D.P.P}|PEPTIDE2{A.C.D.D.P.P}$PEPTIDE1,PEPTIDE2,1:?-?:?$G3(PEPTIDE1+PEPTIDE2:2)$$V2.0

and the bond attributes are;

image

So the display is correct, but confusing. It should show which attributes are not specified. Please note that the HELM notation says that if the ratio is not specified the proportion of that component is unknown and you cannot assume a value (e.g. 1).

scilligence commented 6 years ago

Fixed in 2018-01-08 update. It is able to set the default ratio value by changing the value of this variable:

org.helm.webeditor.defaultbondratio = null; // or ?, 1

ClairePA commented 6 years ago

Good start but there is some work to do before this is fully functioning. The main issues are;

  Action Value Display Preferred Display
1 Draw bond Position = ? null  
    R# = ? null  
    Ratio = 1 Null  
1   Position = ? null  
    R# = ? null  
    Ratio = 1 Null  
  HELM is invalid - see screenshots below.      
2 Go into ‘set attributes' and make no changes. Exit 'set attributes' Position = ? null  
    R# = ? null  
    Ratio = 1 Ratio =1 should be as action1 i.e. the ratio should not be displayed.Â
2   Position = ? null  
    R# = ? null  
    Ratio = 1 Ratio =1  
3 Go into ‘set attributes' and remove the 1 from the ratio. Save and then go back into the 'set attributes' box. The ratio boxes have 1 in them again, this is incorrect - it must be possible to specify an unknown ratio, either by removing the 1 or by adding ?.      
4 Change the value of R# for the first group to 1. Position = ? Pos:?  
    R# = 1 R#: R1  
    Ratio = 1 Ratio =1  
4   Position = ? null Pos:? Should be displayed
    R# = ? null R# should be displayed
    Ratio = 1 Ratio =1  

image image

scilligence commented 6 years ago

Could you write down the steps on how to generate this help? image

ClairePA commented 6 years ago

I can't reproduce the nulls, so please take this off the list of issues.

scilligence commented 6 years ago

fixed in 2018-01-10 update

ClairePA commented 6 years ago

The behaviour still looks the same to me. the issues identified in the comment above are still there. Please implement the changes described in the table and the list below:

scilligence commented 6 years ago

Fixed

ClairePA commented 6 years ago

Perfect! Many thanks.