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

Add ENA-required submission acknowledgements to our submission acknowledgements #2333

Merged
merged 16 commits into from
Jul 30, 2024

Conversation

anna-parker
Copy link
Contributor

@anna-parker anna-parker commented Jul 25, 2024

resolves ##2297

preview URL: https://clarify-submission.loculus.org/

Information about ENA submission in pathoplexus/pathoplexus#47

Summary

Add an additional checkbox which includes promises made to ENA:
image

Screenshot

@anna-parker anna-parker added the preview Triggers a deployment to argocd label Jul 25, 2024
@anna-parker anna-parker marked this pull request as ready for review July 25, 2024 09:48
@theosanderson
Copy link
Member

theosanderson commented Jul 25, 2024

Thanks for this!! A few thoughts:

  • All Pathoplexus sequences reach INSDC so any terms that are inspired by INSDC should not be restricted to Open data terms. We should secondly avoid this to avoid confusing users: the data use terms question should be specifically about "How do you want your data used?" rather than confirming things about your sample. If we need people to do the latter, that should be a subsequent question. If we do this at all, then IMO it should be either part of the existing checkbox in the "Acknowledgement" section, or a second checkbox in that section.
  • Whenever we add checkboxes we should carefully consider if they are definitely needed: it's easy to add overhead that ultimately damages the experience. These conditions already exist as part of our terms of use. Does this checkbox exist as part of the ENA submission process for users not using Pathoplexus? We could also consider making this checkbox part of group creation.
  • I'm not sure if the info button on the Institution is needed. We've already made clear this data is publicly available, and made clear that sequences are submitted to INSDC. IMO that's enough to infer that the Institution will likely be submitted to INSDC along with everything else about the sequence. If we do have this, I think it would be better as a tooltip than something that needs a click.

@anna-parker
Copy link
Contributor Author

anna-parker commented Jul 25, 2024

Good points!

  1. We need to add a page for https://main.loculus.org/docs/concepts/insdc-submission anyways (see [FEAT] Optimize image sizes pathoplexus/pathoplexus#41) -> so maybe we should just add the additional information about center name there. I just think that for submission this is quite important (if we somehow disappear users can still claim their data on INSDC - but the process kinda depends on groups being able to confirm their identity -> that is typically done with center name we were told.
  2. I will move the additional ENA acknowledgements to another check box!

theosanderson and others added 6 commits July 29, 2024 13:15
… and other initContainers (#2340)

* add basic resource requirements for config processor

* specify more resources
* CI for automatic backend formatting on request

* intentionally misformat some code

* Automated backend code formatting

---------

Co-authored-by: Loculus bot <[email protected]>
- Removed unused `namespace` field
- Added server side apply

This was generated by patching a file acquired with `kubectl get appset -o yaml` so formatting is not preserved
@theosanderson
Copy link
Member

(I pushed a small tweak to spacing as it was hard to make as a "suggestion" due to it involving code not directly edited in this PR, hope that's OK)

Copy link
Member

@theosanderson theosanderson left a comment

Choose a reason for hiding this comment

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

This is great - made two more small changes - feel free to disagree with either. Thanks a lot!

@anna-parker anna-parker merged commit a34eed8 into main Jul 30, 2024
14 checks passed
@anna-parker anna-parker deleted the clarify_submission branch July 30, 2024 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Triggers a deployment to argocd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants