Improvements to Container classes #1123
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Writing `ConcentrationContainers` lead to columns named "concentration_concentration_cog", now removed the redundant prefix - same for leakage parameters: use "leakage" prefix, and remove that prefix from the names. - renamed `leakage1_pixel` to `one_pixel_percent` and `leakage1_intensity` to `one_pixel_intensity` to be clearer what they mean
These contain just basic event header data like tel_id and obs_id
the old LeakageContainer had the wrong prefix and confusingly named fields. Now a LeakageContainer becomes the following when turned into columns of an output Table: - leakage_intensity_1pix - leakage_percent_1pix - leakage_intensity_2pix - leakage_percent_2pix
…nto refactor/improve_containers
- Added `DeprecatedField` class to be able to describe fields that will disappear soon - Introduce `EventIndexContainer` and `TelEventIndexContainer` to store event index info (event_id, tel_id, obs_id), and deprecated places where these were used elsewhere - added a MorphologyContainer for morphology parameters (calculation will be added in separate PR) - added a `SimulatedShowerDistribution` container, for storing the sim_telarray thrown shower historgrams.
- fill index containers correctly - fix bug where missing input_url didn't always fil correctly. - allow `~` and env vars in input filenames
maxnoe
reviewed
Sep 30, 2019
Codecov Report
@@ Coverage Diff @@
## master #1123 +/- ##
==========================================
+ Coverage 85.88% 85.89% +<.01%
==========================================
Files 182 182
Lines 11410 11300 -110
==========================================
- Hits 9800 9706 -94
+ Misses 1610 1594 -16
Continue to review full report at Codecov.
|
maxnoe
reviewed
Oct 14, 2019
leakage_fraction_X -> leakage_pixels_X
watsonjj
previously approved these changes
Oct 22, 2019
maxnoe
reviewed
Oct 22, 2019
…nto refactor/improve_containers
- added Deprecated - changed deprecated members to use it - also added a "version" attribute - added test
watsonjj
requested changes
Oct 23, 2019
maxnoe
requested changes
Oct 23, 2019
Ah, yes, that is a problem - perhaps a simple class decorator may be better, but then you get a warning when the Container is constructed, not on access. |
|
Do you have a better suggestion? I could just go back to my original idea: A DeprecatedField class that is a subclass of Field and that raises a warning on access? (I just never got around to adding the warning) |
|
No, unfortunately I couldn't get it to work. |
|
I think we can figure out later how to warn in a nicer way (on use). For now, I think it's fine just to mark the class as deprecated. |
watsonjj
approved these changes
Oct 23, 2019
vuillaut
approved these changes
Oct 25, 2019
maxnoe
approved these changes
Oct 25, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first of a set of PRs that are needed for #1066. These are just some naming cleanups and new containers (not a full redesign of the event structure yet). It includes:
Deprecation of Fields
new
DeprecatedFieldclass to mark fields that are going away soonNormalizing the names of the Concentration and Leakage fields:
Writing
ConcentrationContainerslead to columns named "concentration_concentration_cog", now removed the redundant prefixleakage1_pixeltopixels_width_1(similar forpixels_width_2), and andleakage1_intensitytointensity_width_1to be clearer what they mean. These then get written with the prefix asleakage_intensity_width_1, for exampleAdded an ImageParametersContainer
Just for collecting all of the various parameter sets. May eventually be replaced with a less hard-coded version, but is useful now.
Added EventIndexContainer and TelEventIndexContainer
Store things like
event_id,tel_id, andobs_idin one central place, so you can add them to any event list easily. Deprecated places where they were filled elsewhere.Added a few new containers for later use:
SimulatedShowerDistributioncontainer, for storing the sim_telarray thrown shower histograms.Updated SimTelEventSource
~or environment variables