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

Improve kernel background estimator #739

Merged
merged 3 commits into from Oct 17, 2016

Conversation

Projects
None yet
2 participants
@adonath
Member

adonath commented Oct 12, 2016

This PR is a rework of the KernelBackgroundEstimator class. It includes the following changes:

  • Get rid of the KernelBackroundEstimatorData class and replace its use by SkyImageList
  • Get rid of I/O functionality of the KernelBackgroundEstimator class, as I/O is attached to SkyImageList
  • Make use of SkyImage class everywhere
  • Restructure code into methods and simplify implementation

Some things are left to be done:

  • Adapt notebooks in gammapy-extra
  • Add docs with a short usage example and link to notebooks
  • Move plotting code in docs/tutorials/background/source_diffuse_estimation.py in a show_image_stack() method

@adonath adonath added this to the 0.5 milestone Oct 12, 2016

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Oct 12, 2016

Member

@adonath - Thank you. 👍
I don't have time to review. Please merge whenever you're done here.

Member

cdeil commented Oct 12, 2016

@adonath - Thank you. 👍
I don't have time to review. Please merge whenever you're done here.

@adonath adonath merged commit 1841976 into gammapy:master Oct 17, 2016

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@adonath adonath deleted the adonath:rework_kernel_background_new branch Oct 17, 2016

@cdeil cdeil changed the title from WIP: Rework kernel background estimator to Improve kernel background estimator Nov 18, 2016

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Nov 18, 2016

Member

@adonath - The task Adapt notebooks in gammapy-extra here was still open. Is it done?

Member

cdeil commented Nov 18, 2016

@adonath - The task Adapt notebooks in gammapy-extra here was still open. Is it done?

@adonath

This comment has been minimized.

Show comment
Hide comment
@adonath

adonath Nov 18, 2016

Member

@cdeil I guess it's OK, otherwise the tests wouldn't pass...

Member

adonath commented Nov 18, 2016

@cdeil I guess it's OK, otherwise the tests wouldn't pass...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment