Address error when no gating veto argument was given

[veronica-villa]

When no --gating-veto-windows argument was given the following error arised:

Traceback (most recent call last): File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/pycbc/x86_64_rhel_7/virtualenv/pycbc-v1.16.11/bin/pycbc_multiifo_coinc_findtrigs", line 113, in <module> args.gating_veto_windows) File "/cvmfs/oasis.opensciencegrid.org/ligo/sw/pycbc/x86_64_rhel_7/virtualenv/pycbc-v1.16.11/lib/python2.7/site-packages/pycbc/io/hdf.py", line 1012, in __init__ gating_veto = gating_veto_windows[self.ifo].split(',') AttributeError: 'NoneType' object has no attribute 'split'

This change should address the error

[tdent]

I am not sure if this is the correct fix, was this an error in the coinc findtrigs code if the option is not given?

If so what value is passed to the ReadByTemplate class? Can this not be fixed to be None?

[veronica-villa]

No, this was an error in the hdf code because when the argument is not given the MultiDetOptionAction class returns a default value of {} (which is passed to the ReadByTemplate class) so the argument itself is never None

[tdent]

I.e. it would be good to 1) determine what exactly is currently passed from MultiDetOptionAction if there is no value set on the command line, and 2) determine if the option can be fixed to return None if nothing is given on command line.

[tdent]

OK, so the default is empty dict which is taken to be False (ie if {} returns False). But this is IMO somewhat sloppy since it is not clear from looking at the code that this is what is returned by the option.

[tdent]

Or we have the default initialization of gating_veto_windows in the class to be {} (empty dictionary) and then do a switch like if ifo in gating_veto_windows: (see eg https://stackoverflow.com/questions/1323410/should-i-use-has-key-or-in-on-python-dicts )

[veronica-villa]

Would it be clearer adding

if not args.gating_veto_windows:
    args.gating_veto_windows = None

to the coinc findtrigs code?

[tdent]

OK, I changed my mind since it seems easier to consider gating_veto_windows as a dictionary since that is what is most directly returned by the option parsing.

[koustavchandra]

Thanks @veronica-villa for addressing it in such a short time.

[tdent]

recommendation : change the default __init__ method to set the variable to {} (empty dictionary) and change the switch to if self.ifo in gating_veto_windows.

[tdent]

hmm, I'm not sure if we should do anything about the codeclimate warning. Probably not since we are initializing exactly once and we don't use the gating_veto_windows kwarg value for anything else in the rest of the class.