Allow index-filename to differ from bam_filename + .bai default

[TyberiusPrime]

htslib allows the user to specify an index name that is not 'bam_file.bam.bai'.

This exposes it on IndexReader as ::from_path_and_index.

(I'm very new to rust - any and all feedback is appreciated).

[TyberiusPrime]

Somethings wrong with the CI - those tests passed locally :(.

[johanneskoester]

Indeed, I have the same problem. I have no idea what is wrong though. I am about to leave for a week of vacation. I won't be able to fix this until I am back. Until then, any insights would be greatly appreciated!

[dlaehnemann]

The first error that occurs is a simple one: it complains that (the most recent) cargo fmt was not run on the code. But after that, tests keep on failing and eventually it times out because it doesn't receive input for more than 10 mins. The last one sounds like a CI problem, which might go away in a new run? So maybe fix the cargo fmt and rerun online, as a first step? And the test failures seem to be mostly file reading related (Failed to open file test/test.bam), which could also be a CI infrastructure problem?

[dlaehnemann]

This does remove the cargo fmt error, but everything else seems to remain as is. All the errors have to do with opening files and rust panicking on it. Not sure what's happening.

The testing errors appeared first in:
https://travis-ci.org/rust-bio/rust-htslib/jobs/513004219

And it was still working with:
https://travis-ci.org/rust-bio/rust-htslib/jobs/512916196

I have compared the two setups in quite some detail, and can't spot any differences in versions of anything used. Not in the Travis worker version, not in the rust installation(s), not in the apt sources, not in the crates that cargo downloads.

And the only code that changed in between is the addition of bam index creation code:
79fc2a0...63da354

No clue, sorry...

[dlaehnemann]

Did a lot of digging, and finally found the problem: The pointer action required to interact with the htslib API was not working as expected. All the failing file reads seem to have been collateral damage from that.

A fix is in #125, so once this is merged, you can rebase against the new master and tests should work as expected, again.

[dlaehnemann]

@TyberiusPrime: the test failures should now be fixed by #125 and #126, so you should be able to rebase onto the new master HEAD to get your branch to pass the tests.

[TyberiusPrime]

Here we go.

[coveralls]

Pull Request Test Coverage Report for Build 597

• 30 of 32 (93.75%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.06%) to 48.509%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/bam/mod.rs	30	32	93.75%

Totals
Change from base Build 594:	0.06%
Covered Lines:	13424
Relevant Lines:	27673

---

💛 - Coveralls

[thomasmulvaney]

Just a couple of thoughts in regards to code style. The function from_path_and_index has quite a bit of nesting.

I think it might be easier to read if you do an early return if either of the paths doesnt exist at the top of the function:

if !path.as_ref().exists() || !index.as_ref().exists() {
   return IndexedReaderPathError::InvalidPath
}

...

I would then consider pulling out the strings or returning an error when a None value is encountered:

let p = path.as_ref().to_str().ok_or(IndexedReaderPathError::InvalidPath)?;
let idx_p = ...

The ok_or method turns an Option into an Result.

There should be no nesting then and hopefully easier to follow.

[dlaehnemann]

This and what he said: #124 (comment)

;)

[TyberiusPrime]

Just a couple of thoughts in regards to code style. The function from_path_and_index has quite a bit of nesting.

I think it might be easier to read if you do an early return if either of the paths doesnt exist at the top of the function:

You're of course right. I just stuck to the initial style but have changed it accordingly.

[dlaehnemann]

With the changes that @thomasmulvaney suggested implemented and the tests passing, this looks good to go, for me. Thanks for contribution and the patience with the master branch!