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

Improve 3D analysis code using gammapy.maps #1373

merged 5 commits into from Apr 12, 2018


Copy link

@cdeil cdeil commented Apr 12, 2018

@registerrier - This is a follow-up to #1372 . I plan to merge it tonight, so that tomorrow at 11 we can discuss how to proceed.

Note that the goal is to have a working 3D analysis by tomorrow for a simple case in gammapy.master.
It will then take weeks / months to improve iteratively.

  • Clean up code and docstrings a bit
  • For now, store background and exposure map without units (will be done in #1206 )
  • Use existing fill_map_counts that was added in #1317
  • Rename to gammapy/cube/
  • Add a few automated tests in gammapy/cube/
@cdeil cdeil added the cleanup label Apr 12, 2018
@cdeil cdeil added this to the 0.8 milestone Apr 12, 2018
@cdeil cdeil self-assigned this Apr 12, 2018
Copy link
Member Author

@cdeil cdeil commented Apr 12, 2018

@registerrier - There's many things left to do and improve here. But basically your new functions are working and give the same exposure and background cube like the old functions we had. For background I had to multiply with energy bin width, and for now I had to drop units from for background and exposure, because those aren't supported yet (storing quantity objects in causes all kinds of problems).

So if CI tests pass, I'll merge this in, and then we can continue in follow-up PRs tomorrow.

@cdeil cdeil merged commit 1099765 into gammapy:master Apr 12, 2018
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
continuous-integration/appveyor/pr AppVeyor build succeeded
@registerrier registerrier added this to Done in gammapy.maps Apr 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

1 participant