Proposed replacement for progress PR #1302
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #1302 +/- ##
===============================================
+ Coverage 67.644% 69.301% +1.657%
- Complexity 8188 9321 +1133
===============================================
Files 560 563 +3
Lines 33462 36757 +3295
Branches 5635 6502 +867
===============================================
+ Hits 22635 25473 +2838
- Misses 8644 8958 +314
- Partials 2183 2326 +143
|
…ates are not strictly increasing.
yfarjoun
force-pushed
the
yf_he_progresslogger_rns
branch
from
8c2b4de to
4ad0ffe
Compare
yfarjoun
commented
Feb 25, 2019
| this.lastStartTime = System.currentTimeMillis(); | ||
| protected synchronized boolean record(final String chrom, final int pos, final String rname) { | ||
| lastChrom = chrom; | ||
| if (pos >= lastPos) { |
There was a problem hiding this comment.
only increase counter when chrom name is unchanged, and do not decrease counter.
yfarjoun
commented
Feb 25, 2019
| } | ||
|
|
||
| /** Left pads a string until it is at least the given length. */ | ||
| private String pad (String in, final int length) { | ||
| while (in.length() < length) { | ||
| in = " " + in; | ||
| final StringBuilder inBuilder = new StringBuilder(in); |
There was a problem hiding this comment.
preallocate the builder
There was a problem hiding this comment.
rewrite to avoid silliness.
lbergelson
added
the
Waiting for revisions
5 tasks
added 6 commits
February 25, 2019 22:36
|
@lbergelson this is ready for a review... |
lbergelson
requested changes
Mar 20, 2019
There was a problem hiding this comment.
@yfarjoun This seems much better. A few minor comments and then 👍
| private int lastPos = 0; | ||
| private String lastReadName = null; | ||
| private long countNonIncreasing = 0; | ||
| final private long PRINT_READ_NAME_THRESHOLD = 1000; |
There was a problem hiding this comment.
might as well make this static
| private String pad (String in, final int length) { | ||
| while (in.length() < length) { | ||
| in = " " + in; | ||
| protected static String pad(final String in, final int length) { |
There was a problem hiding this comment.
make this default package instead of protected.
| ("hello", 5, "hello"), | ||
| ("hello", 4, "hello"), | ||
| ("hello", -1, "hello")).foreach { case (in, len, expected) => | ||
| AbstractProgressLogger.pad(in, len) shouldEqual expected |
There was a problem hiding this comment.
The formatting seems really weird here, but maybe that's scala standard?
There was a problem hiding this comment.
yeah.that's the default intellij formatting
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a replacement for #1180. it adds the feature that the read-name printing is only turned on after the logger notices 1000 incidents where the position was decreasing.
More exactly: The logger keep a counter that increments when the position is decreasing and vice versa (but remains non-negative) If this counter is greater than 1000, the logger will also emit read-names.