-
Notifications
You must be signed in to change notification settings - Fork 32
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
GWAS tutorial #629
GWAS tutorial #629
Conversation
The build is failing because the notebook can't import sgkit. I'm not sure how to fix this though. Should the notebook call
|
The build is now failing when trying to execute the notebook, with
and
I can't reproduce on Mac, so I think it may be related to the Linux wheel. See also brentp/cyvcf2#205 |
cf2d4fe
to
e470220
Compare
Codecov Report
@@ Coverage Diff @@
## main #629 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 35 35
Lines 2815 2829 +14
=========================================
+ Hits 2815 2829 +14
Continue to review full report at Codecov.
|
The build was failing when cyvcf2 was reading a remote VCF file, so I spent some time getting the cyvcf2 wheels getting built again in the hope that would fix it. Unfortunately it didn't (brentp/cyvcf2#216), so to make progress on this issue I have changed the notebook to download the remote VCF to a local file before using cyvcf2 to read it. This should be ready for review now. The rendered notebook is here. |
One question: is this a good thing to encourage: from sgkit.io.vcf import vcf_to_zarr Why not just use |
Not sure about phrasing: " so it only loads the first alternate allele " - how about "converts" rather than "loads"? (This is one of the reasons I dislike notebooks - impossible to collaborate on via Git!) |
Actually, is there any reason not to convert this to MyST markdown? I guess the upside of a notebook is that people can download it directly and run it there. This isn't so easy if you go the whole hog with JupyterBook. Maybe there's an intermediate step though? |
Thanks for taking a look @jeromekelleher!
This goes back to the discussion about keeping the IO packages separate (#494). As it stands if you
Agreed - I'll fix this.
Yes, that's why I went with a notebook. Also MyST is relatively new, so I wasn't sure how stable it is yet. |
Right, gotcha. |
I've had a quick look through, and it LGTM. Probably easier to do small edits for any changes, so let's merge whenever you're happy. |
Great to see this go in! @tomwhite should we file an |
Opened #645 |
Fixes #463
Here is the rendered notebook with brief explanatory notes. The notebook output is also included in the site documentation using JupyterBook.