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

Remove `BackgroundModels` class #2359

Merged
merged 12 commits into from Sep 10, 2019

Conversation

@adonath
Copy link
Member

commented Sep 10, 2019

This PR includes the following changes:

  • Remove BackgroundModels class
  • Introduce sort_keys option to write_yaml
  • Move _dict_to_skymodel to SkyModel.from_dict()

I adapted the tests, so that the SkyDiffuseCube is now part of the sky model and not a BackgroundModel anymore.

I'd like to apologize @QRemy for removing so much of the code, that you have written to handle those multiple background models. Introducing the BackgroundModels some time ago, was a mistake that I did and I think it's better if I correct for it now, then to stick with it, because it would create a lot of extra code we had to write in future.

@adonath adonath self-assigned this Sep 10, 2019
@adonath adonath added the cleanup label Sep 10, 2019
@adonath adonath added this to the 0.14 milestone Sep 10, 2019
@cdeil

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

@adonath - Thanks!

I think the saying goes something like this?
"A design is complete when there's nothing left to remove".

@adonath adonath merged commit 2379865 into gammapy:master Sep 10, 2019
9 checks passed
9 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
Scrutinizer Analysis: 1 new issues, 7 updated code elements – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
gammapy.gammapy Build #20190910.5 had test failures
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.