Update nominal values data structure and output

AndrewLevin commented 4 years ago

This pull request fixes the issue of files being accessed prematurely by using a with...as block which defines the scope of the file, so it is automatically flushed and closed at the end of that block.

Description

In addition, this pull request uses a pandas Series to output the nominal DAC values instead of the write() function.

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

This pull request partially resolves https://github.com/cms-gem-daq-project/vfatqc-python-scripts/issues/291. I will make another pull request vfatqc-python-scripts to fix the unclosed files there.

How Has This Been Tested?

Yes, I reran a DAC scan analysis and found that the output NominalValues files were the same as when the HEAD of develop was used.

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.

AndrewLevin commented 4 years ago

The PR seems good. But, it indeed fixes only the most urgent issue with the DAC scans which affects/ed P5 commissioning operations. A full review of all the file accesses will probably be required in the future.

Yes, I will follow up, although I think this is only place where we are actually copying or moving a file inside of a python script -- if we just change the file permissions, it shouldn't matter if the file is flushed, right?

lpetre-ulb commented 4 years ago

Yes, I will follow up, although I think this is only place where we are actually copying or moving a file inside of a python script -- if we just change the file permissions, it shouldn't matter if the file is flushed, right?

Right, changing the file permissions shouldn't matter. And, indeed, your PR fixes the only known issue caused by files not being flushed after a write.

However, depending on how the files and the code are used in the future, every file write is a potential issue... For example, that code in testConnectivity.py is OK since the file is closed. But, I haven't all file writes.

AndrewLevin commented 4 years ago

OK, yes, I see, that would be a problem if the file were not closed.

jsturdy commented 4 years ago

needs a rebase

AndrewLevin commented 4 years ago

rebased

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