[Bug Fix] OH key was used but not set in loop

cms-gem-daq-project / gem-plotting-tools

Repository for GEM commissioning plotting tools

GNU General Public License v3.0

1 stars 26 forks source link

[Bug Fix] OH key was used but not set in loop #254

Closed AndrewLevin closed 4 years ago

AndrewLevin commented 4 years ago

A dictionary of output files in the sbitThresh analysis routine was being incorrectly filled with only one OH key, causing a crash.

Description

A loop over the OHs was being performed, but the OH key was not being set inside the loop, so it was always the last OH key from the previous loop.

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Motivation and Context

Resolves https://github.com/cms-gem-daq-project/gem-plotting-tools/issues/253

How Has This Been Tested?

Yes, I tested it with the example reported here:

https://mattermost.web.cern.ch/cms-gem-daq/pl/mtab9kigg7yytkixet8k91b8ko

Screenshots (if appropriate):

Checklist:

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[ ] I have added tests to cover my changes.
[ ] All new and existing tests passed.

jsturdy commented 4 years ago

We should verify that the expected behaviour is reproduced in each of the following situations:

New data taken, no special arguments (all info comes from the new tree)
New data taken, specify indices, name comes from chamber_config? or name comes from the tree?
Old data taken, specify indices, name comes from chamber_config

It should now fail in the case one is reanalyzing old data, but for which the chamber config is not specified on the command line, otherwise, there will be a possibility to incorrectly take the config from chamber_config based on specified (slot,link,vfat)

AndrewLevin commented 4 years ago

We should verify that the expected behaviour is reproduced in each of the following situations:

New data taken, no special arguments (all info comes from the new tree)

I have tested that this worked.

New data taken, specify indices, name comes from chamber_config? or name comes from the tree?

The name comes from the tree.

Old data taken, specify indices, name comes from chamber_config

The name comes from the chamber_config.

It should now fail in the case one is reanalyzing old data, but for which the chamber config is not specified on the command line, otherwise, there will be a possibility to incorrectly take the config from chamber_config based on specified (slot,link,vfat)

One of the three options for the chamber config is a required argument now.