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

Addresses #180, Implementing Idea #2 #189

Closed bdorney closed 5 years ago

bdorney commented 5 years ago

Description

Addresses #180 by implementing idea number 2.

I've completely removed the chamber_vfatMask dictionary. This was just one more thing that had to be edited and tracked by hand. Software tools exist now to get the VFAT mask on the fly so there's no point in tracking it in a config location. Removes one more potential spot for error.

Types of changes

Motivation and Context

Need to be able to handle multiple uTCA crates and AMC's.

How Has This Been Tested?

Took a new DAC scan and then analyzed data successfully.

Screenshots (if appropriate):

Checklist:

bdorney commented 5 years ago

As long as you're completely reworking the original chamberInfo hacks, may as well simply have the loading modules pull them from a location specified by an environment variable, rather than some fixed location within the package (would play into your QoL style requests)

I'm not sure I understand what you mean by "loading modules" do you mean that we should change chamberInfo.py to load some text file that then fills the dicts and that it should look in some $ENV variable?

jsturdy commented 5 years ago

That's probably less of a change than finding every place that does ("loading modules"):

from chamberInfo import thing

and having it look in some variable dependent location.

What would be best was if it tries to load from the variable location, if it fails, then continues using some default that is provided in the package, printing a message about using defaults (or it could fail, if that is the desired behaviour)

Something like the following (here user_constants.py is in some directory that has been added to the PYTHONPATH variable); the commented lines may be uncommented if one prefers to manipulate the system path on the fly (not the best practice in general), and in that case the environment variable CONSTANTS_PATH should point to the location of user_constants.py):

# import sys,os

try:
    # sys.path.append(os.getenv("CONSTANTS_PATH"))
    from user_constants import GBT
except Exception as e:
    print(e,"Using package default")
    GBT = {
        "VAR1": 1,
        "VAR2": 2,
        "VAR3": 3,
    }

The contents of user_constants.py in this example is:

GBT = {
    "VAR1": 5,
    "VAR2": 4,
    "VAR3": 7,
}
bdorney commented 5 years ago

okay; I would prefer to put this in a different PR.

Can you make a new issue and reference your comment above?

jsturdy commented 5 years ago

95% of the work is already done (and the other 5% is just copy/paste/modify from my previous comment), and you've now added several setup specific configurations, which is the whole point of having a user specifiable location. Better to just "get it done right"...

bdorney commented 5 years ago

@jsturdy was the most recent commit what you intended?