Skip to content
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

Make node schema maxItemsFrom and molecule aware #62

Merged
merged 50 commits into from
May 30, 2022
Merged

Conversation

sverhoeven
Copy link
Member

@sverhoeven sverhoeven commented Apr 5, 2022

Refs #59

To test on small catalog

  1. Start app
  2. select the molaware catalog
  3. fill the molecules array in the global parameters with some PDB files, downloaded from https://www.rcsb.org
  4. Add a node and open its form
  5. The properties should have maxItems equal to the number of molecule files that where uploaded.
  6. The node 3 and 4 use the PDB files to limit the allowed chains and residues in their props. A PDB file with just chain 'A' should only allow 'A' to be selected in the chain property. A PDB files with residue number 1..100 should only allow 1..100 to be selected in the residue property.

To test on haddock catalog

  1. Start app
  2. select the haddock3expert catalog
  3. fill the molecules array in the global parameters with some PDB files, downloaded from https://www.rcsb.org
  4. Add rigidbody node, edit, expand molecule
    • mol_fix_origin has max items of number of molecules
  5. Add flexref node, edit, expand flexibility
    • seg has same number of items as molecules
    • seg start/end have residues

To test validation of upload

  1. Start app
  2. select the haddock3expert catalog
  3. Upload workflow (18).zip which is valid
  4. Upload workflow-bad-resnum.zip which has bad residue number
  5. For validation errors see DevTools console

@netlify
Copy link

netlify bot commented Apr 5, 2022

Deploy Preview for wonderful-noether-53a9e8 ready!

Name Link
🔨 Latest commit 8587a87
🔍 Latest deploy log https://app.netlify.com/sites/wonderful-noether-53a9e8/deploys/62949eabafa01c00088ddab5
😎 Deploy Preview https://deploy-preview-62--wonderful-noether-53a9e8.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sverhoeven sverhoeven changed the title Make node schema maxitemsFrom aware Make node schema maxItemsFrom aware Apr 5, 2022
@sverhoeven sverhoeven changed the title Make node schema maxItemsFrom aware Make node schema maxItemsFrom and molecule aware Apr 14, 2022
Copy link
Contributor

@Peter9192 Peter9192 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For starters, here are my experiences from testing the new functionality. Some of these might be for follow up, others you might want to address here. I have not yet looked at (all) the code, so will follow up with that.

It is difficult to know which setting belongs to which molecule. It might be helpful to display the name/filename or other metadata aboutthe molecule in the builder. E.g.

Specifications for molecules:
  Specification for molecule 5t89:
    Is this molecule a shape?
  Specification for molecule 4hhb:
    Is this molecule a shape?

The selection of residues in molaware catalog -- node 4 is not very intuitive. It works with ctrl/shift, but it might be nicer to have tick boxes or so.

I am able to build a form with start residue number > end residue number. Shouldn't this be invalid?

Occasionally, the text box doesn't render properly (e.g. [object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object]. Toggling visual/text usually solves the issue.

In haddock expert - under rigidbody, got error message that ['ab initio mode'].rair[0] should be array. Initially an empty table is shown, adding an empty row fixes it.

The flexref example has a couple of issues:

  • You need to add (empty) rows to the tables for seg and fle, otherwise the submit button doesn't work. However, the error message appears at the top, far above my scroll position.
  • If I start with one molecule, add flexref settings, then go back and add another molecule, there is no extra array item for seg/fle so I cannot modify settings for the second molecule.
  • If I remove a molecule, and try to modify flexref for the remaining one, I get .flexibility.seg should NOT have more than 1 items, but I cannot remove old items because the second table is no longer rendered.

Upload validation: I get validation errors for both files. Happens in Chrome and Firefox. I expected the valid workflow to succeed. The error messages are very unclear, so something else than the residue number seems to be going wrong.

docs/schema.md Outdated Show resolved Hide resolved
@sverhoeven
Copy link
Member Author

For starters, here are my experiences from testing the new functionality. Some of these might be for follow up, others you might want to address here. I have not yet looked at (all) the code, so will follow up with that.

Thanks for testing out the user interface.

It is difficult to know which setting belongs to which molecule. It might be helpful to display the name/filename or other metadata aboutthe molecule in the builder. E.g.

Specifications for molecules:
  Specification for molecule 5t89:
    Is this molecule a shape?
  Specification for molecule 4hhb:
    Is this molecule a shape?

Can be solved later, created #57

The selection of residues in molaware catalog -- node 4 is not very intuitive. It works with ctrl/shift, but it might be nicer to have tick boxes or so.

Can be solved later, created #72

I am able to build a form with start residue number > end residue number. Shouldn't this be invalid?

At the moment there is no way to specifiy that validation rule. We have a lot of other advanced validation we want to work on later, so for now I created #73

Occasionally, the text box doesn't render properly (e.g. [object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object]. Toggling visual/text usually solves the issue.

I have seen this before, but have not been able to find a way to reproduce it. Created #74

Replies on the rest of your review will follow later.

@sverhoeven
Copy link
Member Author

...

In haddock expert - under rigidbody, got error message that ['ab initio mode'].rair[0] should be array. Initially an empty table is shown, adding an empty row fixes it.

Added workaround to set default to an array.

The flexref example has a couple of issues:

  • You need to add (empty) rows to the tables for seg and fle, otherwise the submit button doesn't work. However, the error message appears at the top, far above my scroll position.

Added workaround to set default to an array.

  • If I start with one molecule, add flexref settings, then go back and add another molecule, there is no extra array item for seg/fle so I cannot modify settings for the second molecule.

You are a thorough tester, that is a usecase I did not think about.
I have added integration test for it, so I can replicate the error, but have not yet fixed it.

  • If I remove a molecule, and try to modify flexref for the remaining one, I get .flexibility.seg should NOT have more than 1 items, but I cannot remove old items because the second table is no longer rendered.

You are a very thorough tester, that is a usecase I did not think about.
Will fix after other integration test

Upload validation: I get validation errors for both files. Happens in Chrome and Firefox. I expected the valid workflow to succeed. The error messages are very unclear, so something else than the residue number seems to be going wrong.

Started #75 for clearer error message.

@sverhoeven
Copy link
Member Author

Spawned #78 .

So all review remarks have been fixed or spawned into new issues/PRs.
Merging this PR to prevent branch of branch PRs.

@sverhoeven sverhoeven merged commit 03f8e34 into main May 30, 2022
@sverhoeven sverhoeven deleted the maxItemsFrom-aware branch May 30, 2022 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants