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

[ADAM-1853] Various improvements to readthedocs documentation. #1893

Merged
merged 5 commits into from Feb 7, 2018

Conversation

5 participants
@heuermh
Member

heuermh commented Jan 29, 2018

Fixes #1853.

@coveralls

This comment has been minimized.

coveralls commented Jan 29, 2018

Coverage Status

Coverage decreased (-0.1%) to 82.564% when pulling a806e5c on heuermh:doc-fixes into adff336 on bigdatagenomics:master.

@AmplabJenkins

This comment has been minimized.

AmplabJenkins commented Jan 29, 2018

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/2617/
Test PASSed.

@heuermh heuermh requested a review from fnothaft Jan 29, 2018

@heuermh heuermh added this to the 0.24.0 milestone Jan 29, 2018

@heuermh

This comment has been minimized.

Member

heuermh commented Jan 29, 2018

Updated checkboxes at #1853 to show what this pull request addresses.

@AmplabJenkins

This comment has been minimized.

AmplabJenkins commented Jan 29, 2018

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/2618/
Test PASSed.

@fnothaft

This comment has been minimized.

Member

fnothaft commented Jan 29, 2018

Nice! Are you still working on the issues left in #1853, or is this as far as you're getting?

@heuermh

This comment has been minimized.

Member

heuermh commented Jan 29, 2018

Are you still working on the issues left in #1853, or is this as far as you're getting?

Let's see ... these ones need help/feedback from you

  • RI inline doc refers to s(i) but I don't see it in the equation
  • Left margin in BQSR read ←the read to observe block looks incorrect
  • Figures on http://adam.readthedocs.io/en/latest/benchmarks/algorithms/ should use GATK4 as a label
  • Is it "GATK" or "the GATK"?
  • GATK in "...more performant than the same algorithms implemented in the GATK. ADAM outperforms the GATK when running..." should be GATK4
  • GATK4 comparison is missing in INDEL Realignment figure

I tried messing around with the CSS but couldn't find a easy way to make code blocks wider without completely overhauling the layout (everything uses static widths).

This one I'll add soon in a separate PR along with new pages for homebrew, conda, and docker

  • Add deploying on EMR doc with example config
@fnothaft

This comment has been minimized.

Member

fnothaft commented Feb 2, 2018

GATK4 comparison is missing in INDEL Realignment figure

GATK4 drops the INDEL realigner.

Otherwise, I'll PR the others over to you shortly.

@fnothaft

This comment has been minimized.

Member

fnothaft commented Feb 2, 2018

RE: Is it "GATK" or "the GATK"?

We're consistently using the GATK:

$ cd docs
$ grep GATK -R . | grep -v "the GATK"
./algorithms/reads.rst:   implementation is fully concordant with the Picard/GATK duplicate
@fnothaft

This comment has been minimized.

Member

fnothaft commented Feb 2, 2018

@heuermh I think all of the ones that needed action from me will be resolved after you merge heuermh#6.

@AmplabJenkins

This comment has been minimized.

AmplabJenkins commented Feb 3, 2018

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/ADAM-prb/2637/
Test PASSed.

@akmorrow13 akmorrow13 merged commit 379dd8d into bigdatagenomics:master Feb 7, 2018

2 checks passed

Codacy/PR Quality Review Good work! A positive pull request.
Details
default Merged build finished.
Details

@heuermh heuermh deleted the heuermh:doc-fixes branch Feb 7, 2018

@heuermh heuermh added this to Completed in Release 0.24.0 Feb 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment