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

Managed variant csv file verification #3060

Merged
merged 15 commits into from Dec 20, 2021
Merged

Managed variant csv file verification #3060

merged 15 commits into from Dec 20, 2021

Conversation

dnil
Copy link
Collaborator

@dnil dnil commented Dec 16, 2021

This PR adds a functionality and updates documentation.

How to test:

  1. how to test it, possibly with real cases/data:
  • try loading a managed variants file with a faulty line, e.g.
## Managed variants from some reliable source
#chromosome;position;end;reference;alternative;category;sub_category;description
14;76548781;76548781;CTGGACC;G;snv;indel;IFT43 indel test
17;48696925;48696925;G;T;snv;snv;CACNA1G intronic test
;;;T;C;snv;snv;silly
7;124491972;124491972;C;A;snv;snv;POT1 test snv
  • ensure uploading a normal example still works

Expected outcome:
The functionality should be working
Take a screenshot and attach or copy/paste the output.
Screenshot 2021-12-17 at 09 15 48

Review:

  • code approved by
  • tests executed by

@coveralls
Copy link

coveralls commented Dec 16, 2021

Coverage Status

Coverage decreased (-0.04%) to 83.578% when pulling f01214a on fix_managed_variant_verif into b5917b7 on main.

@dnil dnil marked this pull request as ready for review December 17, 2021 08:22
@codecov-commenter
Copy link

codecov-commenter commented Dec 20, 2021

Codecov Report

Merging #3060 (54e9934) into main (cea95ac) will increase coverage by 0.04%.
The diff coverage is 80.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3060      +/-   ##
==========================================
+ Coverage   83.71%   83.76%   +0.04%     
==========================================
  Files         286      286              
  Lines       16480    16490      +10     
==========================================
+ Hits        13797    13813      +16     
+ Misses       2683     2677       -6     
Impacted Files Coverage Δ
.../server/blueprints/managed_variants/controllers.py 77.63% <72.72%> (+12.48%) ⬆️
scout/adapter/mongo/managed_variant.py 79.71% <100.00%> (ø)
scout/server/blueprints/managed_variants/forms.py 100.00% <100.00%> (ø)
scout/server/blueprints/cases/controllers.py 77.58% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cea95ac...54e9934. Read the comment docs.

Copy link
Member

@northwestwitch northwestwitch left a comment

Choose a reason for hiding this comment

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

Check out my suggestion!

@@ -124,6 +124,36 @@ The out put files of expansion hunter if preferably annotated with [stranger][st
### Compounds (top 20)
Only interesting when the compound inheritance pattern is required, the list can be very long but is by default cropped to the top 20 highest ranked ones as shown in the heading.

## Managed variants

### Managed variants upload file format
Copy link
Member

Choose a reason for hiding this comment

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

🏅

Copy link
Member

@northwestwitch northwestwitch left a comment

Choose a reason for hiding this comment

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

Works perfectly! 🥇

image

I have a few suggestions:

  • Since we are touching the functionality: improving the looks of this form by placing all elements on one line, with the right spacing and vertical padding.

image

  • In the form above replace CSV file with text file, since it's either tab or semicolon-separated text file

  • Add required to the required fields in the other form, when you add a single variant, since it's crashing as well whenever the right fields are not found
    image

docs/user-guide/variants.md Outdated Show resolved Hide resolved
docs/user-guide/variants.md Outdated Show resolved Hide resolved
docs/user-guide/variants.md Show resolved Hide resolved
@dnil
Copy link
Collaborator Author

dnil commented Dec 20, 2021

Thank you for the review!
I fixed the upload button layout, added the same file-name-updater as for gene panels, and made mandatory fields required on both add and edit forms.

Co-authored-by: Chiara Rasi <rasi.chiara@gmail.com>
@dnil
Copy link
Collaborator Author

dnil commented Dec 20, 2021

Meh, but now the edit is partially broken. I'll check, but perhaps after dinner. 😅

Screenshot 2021-12-20 at 16 53 19

@dnil
Copy link
Collaborator Author

dnil commented Dec 20, 2021

Ok, it was just a small thing, broken with an earlier update..

@dnil dnil merged commit 1ec4daa into main Dec 20, 2021
@northwestwitch northwestwitch deleted the fix_managed_variant_verif branch December 21, 2021 09:08
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.

None yet

4 participants