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

Prune defaults from submitted data #46

Merged
merged 7 commits into from
Mar 24, 2022
Merged

Prune defaults from submitted data #46

merged 7 commits into from
Mar 24, 2022

Conversation

sverhoeven
Copy link
Member

@sverhoeven sverhoeven commented Mar 1, 2022

Fixes #24

To see the defaults being pruned

  1. Goto https://deploy-preview-46--wonderful-noether-53a9e8.netlify.app/
  2. Click Global parameters button.
  3. Expand execution sub form
  4. Switch from Visual to Text mode
  5. Fill required fields
  6. Press submit
  7. In the toml text representation you should not see the cores parameter.
  8. Change the number of cores and submit then cores parameter should show up again.

You can do similar things for the nodes.

@netlify
Copy link

netlify bot commented Mar 1, 2022

✔️ Deploy Preview for wonderful-noether-53a9e8 ready!

🔨 Explore the source changes: d73b885

🔍 Inspect the deploy log: https://app.netlify.com/sites/wonderful-noether-53a9e8/deploys/62345ed93e96d50008ac5de0

😎 Browse the preview: https://deploy-preview-46--wonderful-noether-53a9e8.netlify.app

@sverhoeven sverhoeven requested a review from Peter9192 March 9, 2022 08:50
@Peter9192
Copy link
Contributor

6. Press submit

Can't submit because submitting the global config requires an uploaded molecules file, and I don't have one. Getting the following error: .molecules[0] should be string

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.

Hey @sverhoeven nice recursive function :-)

I noticed that a couple of test functions require a change in the length of array/object props. I don't really understand why this behavior makes sense. I can think of cases where the length is in itself a significant thing, even if the values are all defaults. But perhaps that is not applicable to our use case?

src/store.ts Outdated Show resolved Hide resolved
src/pruner.ts Outdated Show resolved Hide resolved
src/pruner.test.ts Show resolved Hide resolved
src/pruner.test.ts Show resolved Hide resolved
src/pruner.test.ts Outdated Show resolved Hide resolved
src/pruner.ts Outdated Show resolved Hide resolved
@sverhoeven
Copy link
Member Author

  1. Press submit

Can't submit because submitting the global config requires an uploaded molecules file, and I don't have one. Getting the following error: .molecules[0] should be string

You can download a pdb file from https://www.rcsb.org/ for example https://files.rcsb.org/download/5T89.pdb

@sverhoeven
Copy link
Member Author

For f(g(h(x),y),z) in store.ts

Yep it gets cryptic after double nest.
I dont want the functions to know about each other so chaining is not an option.
I moved it to a helper method which calls the functions one line at a time.

src/pruner.ts Outdated Show resolved Hide resolved
src/pruner.ts Show resolved Hide resolved
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.

Thanks for the changes.

@sverhoeven sverhoeven merged commit 6de263c into main Mar 24, 2022
@sverhoeven
Copy link
Member Author

Thanks for reviewing

@sverhoeven sverhoeven deleted the prune-defaults-24 branch March 24, 2022 14:34
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.

Don't render parameters in file which have default value?
2 participants