Ha! I was literally thinking about doing this EXACT thing today during the meeting at some point when I was, erm, clearly not going on a brain tangent. Not sure how you've developed these new mind-probing powers, but be careful in my head, it's perilous in there...

So yes, I like it. Note though: wouldn't it be better to just pass LabelHash a Hashbits or CountingHash from pythonland, rather than having it construct a new one in the __new__ method? That would also avoid having to override the LabelHash class at all in __init__.py

On Wed, May 20, 2015 at 08:07:04PM -0700, Camille Scott wrote:

Ha! I was literally thinking about doing this EXACT thing today during the meeting at some point when I was, erm, clearly not going on a brain tangent. Not sure how you've developed these new mind-probing powers, but be careful in my head, it's perilous in there...

No Comment.

So yes, I like it. Note though: wouldn't it be better to just pass LabelHash a Hashbits or CountingHash from pythonland, rather than having it construct a new one in the `new

Yep.

Now leave me alone, I've got hacking to do :)

Very well, carry on :)

While you are on the line -

if I'm loading labels and tags from disk, all I need to do is run

 check_and_allocate_label(label)

once for each label and then

 link_tag_and_label(tag, label)

for each tag/label pair, correct?

Yerp, that should do it.

Huh. Somewhat to my surprise, that seems to work.

• 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?

The tests failed because of lowered code coverage, due to removal of so many lines of code. @mr-c, could you update the code coverage requirements, please, and then review?

Code coverage was reduced due to new function and code paths added without tests :-)

retest this please, jenkins

Hmm @mr-c looks like a Jenkins problem - out of memory in unrelated tests. Any thoughts?

Anyway, the PR is ready for review.

Maybe your test creates a memory leak prior?

On Wed, May 27, 2015 at 06:35:51PM -0700, Michael R. Crusoe wrote:

Maybe your test creates a memory leak prior?

Maybe? But can't think of what it would be...

whew ready for review @mr-c.

@ctb Lookin' good. I found a couple small things via cpychecker but nothing from your commits.