test various load functions

[ctb]

Responsive to #1037.

[ctb]

• Is it mergeable?
• Did it pass the tests?
• If it introduces new functionality in scripts/ is it tested?
  Check for code coverage with make clean diff-cover
• Is it well formatted? Look at make pep8, make diff_pylint_report,
  make cppcheck, and make doc output. Use make format and manual
  fixing as needed.
• Did it change the command-line interface? Only additions are allowed
  without a major version increment. Changing file formats also requires a
  major version number increment.
• Is it documented in the ChangeLog?
  http://en.wikipedia.org/wiki/Changelog#Format
• Was a spellchecker run on the source code and documentation after
  changes were made?
• Is the Copyright year up to date?

[ctb]

Ready for CR and merge @luizirber @camillescott. Note, it merges cleanly with #1031 with all tests passing, too.

[camillescott]

Thought we were trying to avoid asserts in the C++ code?

[ctb]

On Thu, Jun 04, 2015 at 06:38:49PM -0700, Camille Scott wrote:

@@ -1350,20 +1355,22 @@ void SubsetPartition::merge_from_disk(string other_filename)
diskp = (PartitionID *) (buf + i);
i += sizeof(PartitionID);

•        if (!(*diskp != 0)) {      // sanity check.
  

•            throw khmer_exception();
  

•        }
  

•        assert((*diskp != 0)); // sanity check!
  

Thought we were trying to avoid asserts in the C++ code?

I was unable to think of a way to trigger this in a test, and this is a
reasonable check on things when debugging or modifying other code. I think
we're trying to avoid asserts as things that are actually raised for users, but
I don't see any reason not to leave them in as sanity checks.

[camillescott]

Fair enough!

On Thu, Jun 4, 2015 at 6:46 PM, C. Titus Brown notifications@github.com
wrote:

In lib/subset.cc
#1048 (comment):

@@ -1350,20 +1355,22 @@ void SubsetPartition::merge_from_disk(string other_filename)
diskp = (PartitionID *) (buf + i);
i += sizeof(PartitionID);

•        if (!(*diskp != 0)) {      // sanity check.
  

•            throw khmer_exception();
  

•        }
  

•        assert((*diskp != 0)); // sanity check!
  

On Thu, Jun 04, 2015 at 06:38:49PM -0700, Camille Scott wrote: > @@
-1350,20 +1355,22 @@ void SubsetPartition::merge_from_disk(string
other_filename) > diskp = (PartitionID _) (buf + i); > i +=
sizeof(PartitionID); > > - if (!(_diskp != 0)) { // sanity check. > - throw
khmer_exception(); > - } > + assert((*diskp != 0)); // sanity check!
Thought we were trying to avoid asserts in the C++ code?
I was unable to think of a way to trigger this in a test, and this is a
reasonable check on things when debugging or modifying other code. I think
we're trying to avoid asserts as things that are actually raised for users,
but I don't see any reason not to leave them in as sanity checks.

—
Reply to this email directly or view it on GitHub
https://github.com/dib-lab/khmer/pull/1048/files#r31782427.

Camille Scott

Department of Computer Science
Lab for Data Intensive Biology
University of California, Davis

camille.scott.w@gmail.com

[camillescott]

Fine fine, keep your extraneous statements ;P LGTM; merging