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

Use dataset GTI table in LightCurveEstimator #2561

Merged
merged 2 commits into from Nov 19, 2019

Conversation

@registerrier
Copy link
Contributor

registerrier commented Nov 19, 2019

Description
The time information of a datasetis now stored on a GTI table. The LightCurveEstimator uses a hack where the t_start t_stop information is stored on the dataset.counts.meta. See issue #2547.
This pull request solves this by making LightCurveEstimator use the dataset.GTI information for the definition of the time bin.
The PR is paid-coded with @JouvinLea at the Granada coding sprint.

Dear reviewer

Please merge my PR.

@registerrier registerrier self-assigned this Nov 19, 2019
@registerrier registerrier added this to To do in gammapy.time via automation Nov 19, 2019
@registerrier registerrier added this to the 0.15 milestone Nov 19, 2019
Copy link
Member

adonath left a comment

Thanks @registerrier and @JouvinLea, I have no further comments. Let's wait for Travis-CI and merge...

@codecov

This comment has been minimized.

Copy link

codecov bot commented Nov 19, 2019

Codecov Report

Merging #2561 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2561   +/-   ##
=======================================
  Coverage   91.31%   91.31%           
=======================================
  Files         144      144           
  Lines       16208    16208           
=======================================
  Hits        14801    14801           
  Misses       1407     1407
Impacted Files Coverage Δ
gammapy/time/lightcurve_estimator.py 95.45% <ø> (ø) ⬆️
gammapy/data/gti.py 98.52% <100%> (ø) ⬆️

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 9f86caf...cbe017f. Read the comment docs.

@registerrier

This comment has been minimized.

Copy link
Contributor Author

registerrier commented Nov 19, 2019

All tests passed. Merging.

@registerrier registerrier merged commit 1957c8d into gammapy:master Nov 19, 2019
12 checks passed
12 checks passed
greeting
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
Scrutinizer Analysis: No new issues – Tests: passed
Details
codecov/patch 100% of diff hit (target 91.31%)
Details
codecov/project 91.31% (+0%) compared to 9f86caf
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
gammapy.gammapy Build #20191119.5 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 19, 2019
@adonath adonath changed the title Using dataset GTI table in LightCurveEstimator Use dataset GTI table in 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
2 participants
You can’t perform that action at this time.