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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hillas parameters in telescope frame #1591
Hillas parameters in telescope frame #1591
Changes from 37 commits
4028688
7d83987
b52bf0c
4000a08
6aa2eb2
f554d8f
4f61b73
3863add
34f9d5f
9fb0481
058746a
9433e77
8f9a81a
3bd826b
8ded4e7
2647f8a
dbb0f23
b413f85
ecf61e1
f20d04a
4dc2243
0275401
2b108b0
c03e5c0
d6641e2
bfa7505
ff648fb
46ae892
98b7794
6a0f5d4
b281696
ffb8de1
54f2e06
19a30cc
52a46b6
958283f
91fdeba
42ce511
25cca1a
a62e95f
85e0659
6f7c5bc
dee99b0
24755f3
2e33e00
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a Traitlet, not a construction option. Otherwise it is not configurable. The default should be set to True.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That will not change the data model - it is only a configuration option, and the current data model supports both units in the output. However, we may in the future deprecate the CameraFrame one and clean up all that code as I said before, but now we want to be able to run the full analysis twice on a large dataset and ensure they results are compatible with both frames.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we are at it, please also remove the
is_simulation
option. It should not be needed. Code that needs to check that should just checkif event.simulation is None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, not really connected. Will just make a PR