-
Notifications
You must be signed in to change notification settings - Fork 296
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
remake of #909 w/tests #1067
remake of #909 w/tests #1067
Conversation
Conflicts: scripts/abundance-dist.py
Copied over from #909 (putting in comment so as to set up automatic link) |
OK, took me a while to wrap my head around this. The complication is that abundance-dist is used with countgraphs created by load-into-counting. So we end up with the following states:
I would argue that the first behavior is clearly insane and unexpected, while the third behavior is at least expected if not clearly sane. BUT we are now running into the problem that the default behavior of abundance-dist is to have BIGCOUNT default to "yes", which will trigger the insane case :). So, @bocajnotnef, could you add a warning message printed out at the beginning (and maybe end?) of the script, for the situation where load-into-counting has produced a countgraph with BIGCOUNT=no? You can use |
Warning as in "Don't do that, it's bad, sys.exit(1)" or warning as in On Sun, Jun 7, 2015, 18:29 C. Titus Brown [email protected] wrote:
|
2nd! Titus Brown, [email protected]
|
retest this please |
@ctb Added the things. |
Um, how about checking to see if the loaded counting table has bigcount set to no and only warning then? Also, ChangeLog. And checklist :) |
This makes me happy. :) |
ping @bocajnotnef - no hurry, but still stuff to do. |
@ctb Weird thing: Poking around in abundance-dist shows: Does that not override in the way that we expect, and that's the source of the problem? |
abundance-dist does the reporting, load-into-counting does the counting. You can't report higher than 255 if you haven't counted higher than 255. Happy to give you a test that screws up if you like? |
That's alright. Just making sure I had the bits that concern me understood. On Sat, Jun 13, 2015, 15:29 C. Titus Brown [email protected] wrote:
|
retest this please |
Conflicts: scripts/load-into-counting.py
|
@ctb Did the stuff. Thing of note: Don't know a good way to add a reminder at the end of the script but the output is so minimal I don't think it matters. |
Conflicts: ChangeLog tests/test_scripts.py
thx, @bocajnotnef ! |
@ctb moved the pull here as I don't have write access to @drtamermansour's repo.
Check for code coverage with
make clean diff-cover
make pep8
,make diff_pylint_report
,make cppcheck
, andmake doc
output. Usemake format
and manualfixing as needed.
without a major version increment. Changing file formats also requires a
major version number increment.
http://en.wikipedia.org/wiki/Changelog#Format
changes were made?