-
Notifications
You must be signed in to change notification settings - Fork 13
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
Towards using Pyirf #79
Conversation
Co-authored-by: adonini <alice.donini@ts.infn.it>
Co-authored-by: adonini <alice.donini@ts.infn.it>
Co-authored-by: adonini <alice.donini@ts.infn.it>
Co-authored-by: adonini <alice.donini@ts.infn.it>
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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.
Many of the files complain that there is no newline at the end.
Honestly, I don't know if this is important, but just to be sure please add them so at least the red signs disappear
Also, maybe add a couple of lines in the docs under |
We will update the docs in the next PR in which we plan to add the script to produce the performance using pyirf |
Co-authored-by: adonini <alice.donini@ts.infn.it>
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.
Remember to update the EventDisplay main part from pyirf example once the new release is out, because we have recently updated it
Yes we will do it in the next PR because here we are not uploaded the make_performance_pyirf file |
Sorry my bad - I had forgot the scope of this PR |
Co-authored-by: adonini <alice.donini@ts.infn.it>
Co-authored-by: adonini <alice.donini@ts.infn.it>
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.
Please, first merge master into this PR and try to solve the (merge conflicts, hopefully, few and small).
Generally your changes should supersede mine.
I left here and there various changes to revert/add.
If you have any doubt, please contact me!
Codecov Report
@@ Coverage Diff @@
## master #79 +/- ##
==========================================
- Coverage 30.91% 30.78% -0.13%
==========================================
Files 20 20
Lines 2258 2267 +9
==========================================
Hits 698 698
- Misses 1560 1569 +9
Continue to review full report at Codecov.
|
Related to first point in #73