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

Update Gammapy to match Astropy region changes #1088

Merged
merged 2 commits into from Aug 17, 2017
Merged

Conversation

@joleroi
Copy link
Contributor

@joleroi joleroi commented Jul 24, 2017

Test are still running on regions 2.0 I guess, these are some updates to be able to run with regions HEAD.

@joleroi joleroi added the tests label Jul 24, 2017
@cdeil cdeil added this to the 0.7 milestone Jul 28, 2017
@cdeil
cdeil approved these changes Jul 28, 2017
Copy link
Member

@cdeil cdeil left a comment

@joleroi - I left one inline comment.
Otherwise 👍 (if it works, haven't tested with regions master version), please merge.

if self.background_model is not None:
mu_on = prediction[0] + prediction[1]
on_stat = stats.cash(n_on=obs.on_vector.data.data.value,
mu_on=mu_on)
on_stat = stats.__dict__[self.stat](n_on=obs.on_vector.data.data.value,

This comment has been minimized.

@cdeil

cdeil Jul 28, 2017
Member

This is not so nice. I would suggest to at least not do this three times, and instead do this once at the top of the function:

stats_func = getattr(stats, self.stat)

and then call stats_func on the three lines.

@joleroi joleroi force-pushed the joleroi:regions branch from 5aa280f to 6503521 Aug 17, 2017
@joleroi joleroi merged commit a642819 into gammapy:master Aug 17, 2017
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@joleroi joleroi deleted the joleroi:regions branch Aug 17, 2017
@cdeil cdeil changed the title Updates for regions HEAD Update Gammapy to match Astropy region changes Aug 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants