BouchardLab / process_nwb

Functions for preprocessing timeseries data stored in the NWB format
https://process-nwb.readthedocs.io/en/latest/
4 stars 6 forks source link

fix --all_filters #35

Closed hlillemark closed 3 years ago

hlillemark commented 3 years ago

right now --all_filters param sets all_filters to false if it exists. Then hg_only = ~args.all_filters, which is the bitwise not command. ~True = -2, and ~False = -1, so in the current state, both are nonzero and it is always running on hg_only mode. Also it does not need to be negated because hg_only should be false if all filters should be allowed

codecov[bot] commented 3 years ago

Codecov Report

Merging #35 (ac78a18) into master (f720100) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #35   +/-   ##
=======================================
  Coverage   85.46%   85.46%           
=======================================
  Files           7        7           
  Lines         289      289           
=======================================
  Hits          247      247           
  Misses         42       42           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f720100...ac78a18. Read the comment docs.

JesseLivezey commented 3 years ago

This is a bug, but I think the better change is to fix the argparse line

parser.add_argument('--all_filters', action='store_false')

should have been

parser.add_argument('--all_filters', action='store_true')

hg_only should be the negation of all_filters, I just set the wrong default for all_filters.

This would be functionally equivalent to your change, but inline with the variable names.