Configuration parameter for pulse series name #168
Conversation
mlincett
changed the title
🥳 |
I would agree, but when I tried to install the package in a Python 3.11 environment some WIPAC dependencies were not available. I would stick to 3.10 style for this PR and see about moving to 3.11 in a new issue. |
| pulsesName : str, | ||
| cache_dir : str, | ||
| GCD_dir : str, |
There was a problem hiding this comment.
Please excuse the pedantry for a moment. For pep8-compliance, there should be no space before :, like pulsesName: str instead of pulsesName : str. Looks like there are several places with this.
There was a problem hiding this comment.
No worries, but how come black 23.1 is not enforcing this?
There was a problem hiding this comment.
Not even autopep8 (2.0.2, pycodestyle 2.10.0) suggests changes. I took care of it manually.
There was a problem hiding this comment.
# fmt: off is at the top of the file. I put that there to limit needless diffs
There are many hardcoded occurrences of
pulsesName = SplitUncleanedInIcePulses, i.e. the name identifying the pulse series used in the reconstruction.Note that
prepare_frames()used to support the passing of a differentpulsesNamebut in a bad way: the user-selected pulse series would be used to overwrite the data ofSplitUncleanedInIcePulses, so to match all the hardcoded values downstream.With this PR:
pulsesNameis specified in config.py ascfg.INPUT_PULSES_NAME;pulsesNamebecomes an optional argument ofextract_json_message()defaulting to the value specified inconfig;extract_json_message();pepare_frames()no longer supports overriding/overwriting the default pulse series.PR is draft for now
, as I am evaluating the opportunity of making the pulse series dependent onreco_algo. (Also, need to fix failing checks.)Comments are welcome.
UPDATE: at this moment, it makes sense to make Skymap Scanner stick to
SplitUncleanedInIcePulsesas an input, and take care of pulse cleaning as part of the frame preparation / reco segments.