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

Feature docs migration: Round II: Three macros and fitting.fitScanData #123

Closed lmoureaux closed 6 years ago

lmoureaux commented 6 years ago

Description

Add the documentation for:

Types of changes

Motivation and Context

94

How Has This Been Tested?

Docs produced and viewed in man and a Web browser. Build available here.

Checklist:

bdorney commented 6 years ago

For clusterAnaScurve.py the following options are not reported:

  Options for channel mask decisionsParameters which specify how Dead, Noisy, and High Pedestal Channels are charaterized:
    --maxEffPedPercent=maxEffPedPercent
                        Percentage, Threshold for setting the HighEffPed mask
                        reason, if channel (effPed > maxEffPedPercent * nevts)
                        then HighEffPed is set
    --highNoiseCut=highNoiseCut
                        Threshold for setting the HighNoise maskReason, if
                        channel (scurve_sigma > highNoiseCut) then HighNoise
                        is set
    --deadChanCutLow=deadChanCutLow
                        If channel (deadChanCutLow < scurve_sigma <
                        deadChanCutHigh) then DeadChannel is set
    --deadChanCutHigh=deadChanCutHigh
                        If channel (deadChanCutHigh < scurve_sigma <
                        deadChanCutHigh) then DeadChannel is set

Most likely they did not make their way into the README.md but right now would be a good time to add them.

We should also make some statement that the immediate subdirectories of $DATA_PATH should be expected to be the GEMINImXLY directories which was not readily apparent previously. User reported this was not known which indirectly leads to https://github.com/cms-gem-daq-project/gem-plotting-tools/issues/124 (the real fix should include some error message in the script of course but putting info into the documentation will also help!).

bdorney commented 6 years ago

packageFiles4Docker.rst

I assume this is a typo and refers to packageFiles4Docker.py?

bdorney commented 6 years ago

I didn't have a look at the added documentation for the others:

gemTreeDrawWrapper.py
packageFiles4Docker.py
fitting.fitScanData

But my overall comment would be to xcheck the help menu of the actual script to make sure what is included in the documentation reflects that.

Also due to laziness of imports we may have some options that do nothing (since we use a common options menu for several scripts). This is certainly the case in vfatqc. If this is the case here we should try not to have those in the documentation (not sure if that is possible).

lmoureaux commented 6 years ago

Probably forgot to refresh the files in my editor after a git rebase. Will fix.

lmoureaux commented 6 years ago

Removed all code changes from history.

lmoureaux commented 6 years ago

For clusterAnaScurve.py the following options are not reported:

The documentation says:

Finally clusterAnaScurve.py can also be passed the cut values used in assigning a maskReason described at [Providing Cuts for maskReason at Runtime]().

I copied this from the README, but adding the options is a 10min change.

bdorney commented 6 years ago

I copied this from the README, but adding the options is a 10min change.

Great, so I trust you'll be able to quickly implement it.

lmoureaux commented 6 years ago

But my overall comment would be to xcheck the help menu of the actual script to make sure what is included in the documentation reflects that.

Done.

Also due to laziness of imports we may have some options that do nothing (since we use a common options menu for several scripts). This is certainly the case in vfatqc. If this is the case here we should try not to have those in the documentation (not sure if that is possible).

Easy, just don't include it in the docstring (it will still be shown by -h)

bdorney commented 6 years ago

travis had a time out issue again. Could you re-start 156.6?

Probably everything is fine; but should xcheck before merging.