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

Add time intervals to LightCurveEstimator #2567

Merged
merged 9 commits into from Nov 21, 2019

Conversation

@JouvinLea
Copy link
Contributor

JouvinLea commented Nov 19, 2019

This PR introduced given time intervals to compute the LC!

Description

This pull request introduces a time_intervals argument on the LightCurveEstimator. This allows to extract light curves with different time binning over the same Datasets.

The main requirement is that a dataset.GTI in the Datasets is fully contained in a time_interval to be taken into account for the flux extraction.

This PR is intended to resolve issue #2548.

Dear reviewer

@codecov

This comment has been minimized.

Copy link

codecov bot commented Nov 19, 2019

Codecov Report

Merging #2567 into master will increase coverage by 0.01%.
The diff coverage is 98.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2567      +/-   ##
==========================================
+ Coverage    91.5%   91.52%   +0.01%     
==========================================
  Files         140      140              
  Lines       15971    16017      +46     
==========================================
+ Hits        14615    14660      +45     
- Misses       1356     1357       +1
Impacted Files Coverage Δ
gammapy/time/lightcurve_estimator.py 96.15% <98.57%> (+0.69%) ⬆️
gammapy/data/observations.py 77.73% <0%> (ø) ⬆️

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 dae94c5...dcb6d2e. Read the comment docs.

@cdeil cdeil added the feature label Nov 19, 2019
@cdeil cdeil added this to To do in gammapy.time via automation Nov 19, 2019
@cdeil cdeil added this to the 0.15 milestone Nov 19, 2019
@registerrier registerrier changed the title Add time intervals to the LC Add time intervals as an argument to the LIghtCurveEstimator Nov 19, 2019
gammapy/time/lightcurve_estimator.py Outdated Show resolved Hide resolved
gammapy/time/lightcurve_estimator.py Show resolved Hide resolved
gammapy/time/lightcurve_estimator.py Outdated Show resolved Hide resolved
gammapy/time/lightcurve_estimator.py Show resolved Hide resolved
gammapy/time/lightcurve_estimator.py Outdated Show resolved Hide resolved
gammapy/time/tests/test_lightcurve.py Outdated Show resolved Hide resolved
gammapy/time/tests/test_lightcurve.py Show resolved Hide resolved
gammapy/time/tests/test_lightcurve.py Show resolved Hide resolved
gammapy/time/tests/test_lightcurve.py Show resolved Hide resolved
gammapy/time/tests/test_lightcurve.py Outdated Show resolved Hide resolved
@JouvinLea JouvinLea force-pushed the JouvinLea:adding_timeintervals_forLC branch from b27bb1b to 6177f51 Nov 21, 2019
@registerrier

This comment has been minimized.

Copy link
Contributor

registerrier commented Nov 21, 2019

Some comments on the notebook:

  • strip the output
  • Light Curve estimation: by night, rather than perform LC by night
  • You'd rather hard code the time intervals, it is clearer than the piece of code to get them
  • Move Time import to beginning of the notebook
JouvinLea added 5 commits Nov 19, 2019
# The first commit's message is:
Add time intervals to the LC

# This is the 2nd commit message:

improve the tests + remove output notebook

# This is the 3rd commit message:

add counts and ul computation using dataset.mask
@JouvinLea JouvinLea force-pushed the JouvinLea:adding_timeintervals_forLC branch from 6177f51 to df6c1f4 Nov 21, 2019
Copy link
Contributor

registerrier left a comment

Thanks @JouvinLea ! Waiting for Travis-CI to finish and merge.

@registerrier registerrier merged commit fd28ba1 into gammapy:master Nov 21, 2019
9 of 10 checks passed
9 of 10 checks passed
greeting
Details
Codacy/PR Quality Review Not up to standards. This pull request quality could be better.
Details
Scrutinizer Analysis: No new issues – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
gammapy.gammapy Build #20191121.18 succeeded
Details
gammapy.gammapy (DevDocs) DevDocs succeeded
Details
gammapy.gammapy (Lint) Lint succeeded
Details
gammapy.gammapy (Test Python36) Test Python36 succeeded
Details
gammapy.gammapy (Test Windows36) Test Windows36 succeeded
Details
gammapy.gammapy (Test Windows37) Test Windows37 succeeded
Details
gammapy.time automation moved this from To do to Done Nov 21, 2019
@adonath adonath changed the title Add time intervals as an argument to the LIghtCurveEstimator Add time intervals to LightCurveEstimator Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
gammapy.time
  
Done
3 participants
You can’t perform that action at this time.