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

Cleanup mask safe handling #2551

merged 12 commits into from Nov 16, 2019


Copy link

adonath commented Nov 16, 2019

This PR unifies and cleans up the offset-cut and mask_safe handling for MapDataset. It includes the following changes:

  • Remove duplicated configuration from MapDatasetMaker. This means instead of passing the
    same config from MapDataset.create(), a reference dataset is passed to instead
  • Remove the application of the offset cut in MapDatasetMaker. This addresses issue #2451. The offset cut is now fully handled with the mask_safe as well. This has the advantage of taking into account PSF leakage and also give the freedom to change the offset-cut after data reduction and re-stacking the datasets
  • Remove application of the offset-cut from make_edisp_map and make_psf_map
  • The mask_safe is now only applied in MapDataset.stack(), MapDataset.to_image()

Dear reviewer
I plan to work on two follow-up PRs:

  • Introduce the a separate MapDatasetMaker(cutout_width=) argument
  • Add documentation on the data reduction to gammapy/docs/spectrum and gammapy/docs/cube
@adonath adonath self-assigned this Nov 16, 2019
@adonath adonath added the cleanup label Nov 16, 2019
@adonath adonath added this to the 0.15 milestone Nov 16, 2019
@adonath adonath force-pushed the adonath:cleanup_mask_safe_handling branch from 8e0e2f0 to 18a19ea Nov 16, 2019

This comment has been minimized.

Copy link

codecov bot commented Nov 16, 2019

Codecov Report

Merging #2551 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2551      +/-   ##
- Coverage   91.27%   91.26%   -0.01%     
  Files         144      144              
  Lines       16300    16282      -18     
- Hits        14878    14860      -18     
  Misses       1422     1422
Impacted Files Coverage Δ
gammapy/maps/ 95.11% <ø> (ø) ⬆️
gammapy/cube/ 95% <100%> (-0.54%) ⬇️
gammapy/cube/ 89.81% <100%> (+0.26%) ⬆️
gammapy/cube/ 96.37% <100%> (-0.44%) ⬇️
gammapy/cube/ 95.83% <100%> (ø) ⬆️
gammapy/analysis/ 94.21% <100%> (ø) ⬆️
gammapy/maps/ 96.97% <100%> (+0.07%) ⬆️
gammapy/cube/ 95.14% <100%> (-0.51%) ⬇️

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 154ea23...18a19ea. Read the comment docs.

@adonath adonath merged commit a28028c into gammapy:master Nov 16, 2019
12 checks passed
12 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Scrutinizer Analysis: 3 new issues, 4 updated code elements – Tests: passed
codecov/patch 100% of diff hit (target 91.27%)
codecov/project Absolute coverage decreased by -<.01% but relative coverage increased by +8.72% compared to 154ea23
continuous-integration/travis-ci/pr The Travis CI build passed
gammapy.gammapy Build #20191116.5 succeeded
gammapy.gammapy (DevDocs) DevDocs succeeded
gammapy.gammapy (Lint) Lint succeeded
gammapy.gammapy (Test Python36) Test Python36 succeeded
gammapy.gammapy (Test Windows36) Test Windows36 succeeded
gammapy.gammapy (Test Windows37) Test Windows37 succeeded
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
1 participant
You can’t perform that action at this time.