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

make FASTQ reader remove phred bias from quals #4415

Merged
merged 1 commit into from
Feb 15, 2018

Conversation

tedsharpe
Copy link
Contributor

No description provided.

Copy link
Contributor

@mwalker174 mwalker174 left a comment

Choose a reason for hiding this comment

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

Looks straight-forward. Did you want to write a unit test for this?

@SHuang-Broad SHuang-Broad self-requested a review February 14, 2018 19:53
@tedsharpe
Copy link
Contributor Author

OK. That would be the right thing to do.

Copy link
Contributor

@SHuang-Broad SHuang-Broad left a comment

Choose a reason for hiding this comment

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

:+1
Suggestion: rename the corresponding field in FastqRead as phredQuals?

@codecov-io
Copy link

codecov-io commented Feb 14, 2018

Codecov Report

Merging #4415 into master will increase coverage by 0.033%.
The diff coverage is 54.839%.

@@              Coverage Diff               @@
##              master    #4415       +/-   ##
==============================================
+ Coverage     79.017%   79.05%   +0.033%     
- Complexity     16447    16449        +2     
==============================================
  Files           1047     1047               
  Lines          59186    59189        +3     
  Branches        9672     9672               
==============================================
+ Hits           46767    46789       +22     
+ Misses          8664     8639       -25     
- Partials        3755     3761        +6
Impacted Files Coverage Δ Complexity Δ
.../hellbender/tools/spark/sv/utils/SVFastqUtils.java 72.139% <54.839%> (+7.493%) 7 <2> (+2) ⬆️
...bender/tools/walkers/mutect/FilterMutectCalls.java 95.833% <0%> (ø) 7% <0%> (ø) ⬇️
...itute/hellbender/tools/walkers/mutect/Mutect2.java 92% <0%> (ø) 15% <0%> (ø) ⬇️
...park/sv/discovery/alignment/AlignmentInterval.java 89.655% <0%> (+0.383%) 73% <0%> (ø) ⬇️
...nder/utils/runtime/StreamingProcessController.java 70.37% <0%> (+0.823%) 50% <0%> (ø) ⬇️
...oadinstitute/hellbender/utils/gcs/BucketUtils.java 80% <0%> (+1.29%) 39% <0%> (ø) ⬇️

@tedsharpe
Copy link
Contributor Author

added round-trip test that fails without the fix.
responded to rename request with explicit documentation instead.

@tedsharpe tedsharpe merged commit 720eb04 into master Feb 15, 2018
@tedsharpe tedsharpe deleted the tws_fastq_reader_qual_bias branch February 15, 2018 21:01
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.

4 participants