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

Clean up maps/ #2076

merged 6 commits into from Mar 11, 2019

Clean up maps/ #2076

merged 6 commits into from Mar 11, 2019


Copy link

@adonath adonath commented Mar 8, 2019

This PR contains the following changes:

  • Rename WcsGeom.shape to WcsGeom.shape_axes, because the original name was misleading.
  • Restructure and simply WcsGeom.get_pix()
  • Remove untested and unused functions
@adonath adonath added the cleanup label Mar 8, 2019
@adonath adonath added this to the 0.11 milestone Mar 8, 2019
@adonath adonath self-assigned this Mar 8, 2019
@adonath adonath added this to To do in gammapy.maps via automation Mar 8, 2019
@adonath adonath force-pushed the map_geom_footprint branch from 5d1f962 to b177696 Mar 10, 2019
@adonath adonath force-pushed the map_geom_footprint branch from b177696 to 6380ed6 Mar 11, 2019
@@ -43,7 +43,7 @@ def test_simulate():

assert isinstance(dataset, MapDataset)
assert isinstance(dataset.model, SkyModel)
assert isinstance(dataset.model, SkyModels)
Copy link

@kbruegge kbruegge Mar 11, 2019

I'm a bit confused about the naming here. So dataset.model is a SkyModels object containing multiple models? Maybe dataset.models is clearer?

Copy link
Member Author

@adonath adonath Mar 11, 2019

I agree this might be clearer. I'll prepare a PR this week to clean up the Dataset classes before the v0.11 release, which will include a few other changes we've discussed (e.g. renaming Dataset.likelihood() to Dataset.fit_statistic() and improving the docstrings).

@adonath adonath merged commit 75a4558 into gammapy:master Mar 11, 2019
8 checks passed
gammapy.maps automation moved this from To do to Done Mar 11, 2019
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

2 participants