Skip to content
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

Merged
merged 10 commits into from Dec 11, 2020
Merged

Towards using Pyirf #79

merged 10 commits into from Dec 11, 2020

Conversation

gaia-verna
Copy link
Collaborator

Related to first point in #73

gaia-verna and others added 4 commits November 5, 2020 16:40
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>
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Member

@HealthyPear HealthyPear left a 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

@HealthyPear HealthyPear added benchmarks enhancement New feature or request input / output new features or issues regarding input and output formats labels Nov 5, 2020
@HealthyPear HealthyPear added this to In progress in Pipeline features and enhancements via automation Nov 5, 2020
@HealthyPear HealthyPear added this to the Release 0.5 milestone Nov 5, 2020
@HealthyPear
Copy link
Member

Also, maybe add a couple of lines in the docs under perf about these changes

@HealthyPear HealthyPear mentioned this pull request Nov 5, 2020
4 tasks
@gaia-verna
Copy link
Collaborator Author

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>
Copy link
Member

@HealthyPear HealthyPear left a 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

environment.yml Outdated Show resolved Hide resolved
Pipeline features and enhancements automation moved this from In progress to Review in progress Nov 6, 2020
@gaia-verna
Copy link
Collaborator Author

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

@HealthyPear
Copy link
Member

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>
environment.yml Outdated Show resolved Hide resolved
Copy link
Member

@HealthyPear HealthyPear left a 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
Copy link

codecov bot commented Dec 11, 2020

Codecov Report

Merging #79 (9f67772) into master (d7a9b90) will decrease coverage by 0.12%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            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     
Impacted Files Coverage Δ
protopipe/perf/cut_optimisation.py 9.68% <0.00%> (+0.08%) ⬆️
protopipe/perf/irf_maker.py 14.49% <0.00%> (ø)
protopipe/scripts/make_performance.py 10.75% <0.00%> (ø)
protopipe/scripts/write_dl2.py 5.63% <0.00%> (-0.27%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7a9b90...9f67772. Read the comment docs.

Pipeline features and enhancements automation moved this from Review in progress to Reviewer approved Dec 11, 2020
@gaia-verna gaia-verna closed this Dec 11, 2020
Pipeline features and enhancements automation moved this from Reviewer approved to Done Dec 11, 2020
@HealthyPear HealthyPear reopened this Dec 11, 2020
Pipeline features and enhancements automation moved this from Done to In progress Dec 11, 2020
@HealthyPear HealthyPear merged commit f0d73e6 into cta-observatory:master Dec 11, 2020
Pipeline features and enhancements automation moved this from In progress to Done Dec 11, 2020
@gaia-verna gaia-verna deleted the pyirf branch December 12, 2020 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmarks enhancement New feature or request input / output new features or issues regarding input and output formats
Development

Successfully merging this pull request may close these issues.

None yet

2 participants