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

Refactor class CrabSpectrum in a function #2278

Merged
merged 2 commits into from Jul 11, 2019

Conversation

@JouvinLea
Copy link

@JouvinLea JouvinLea commented Jul 10, 2019

This PR is supposed to fix this issue #1788. Tell me if this was what you had in mind.

Once this one merge, I will try to fix this one defining also a test case (#2213)

@JouvinLea JouvinLea requested a review from adonath Jul 10, 2019
@adonath adonath self-assigned this Jul 10, 2019
@adonath adonath added this to the 0.13 milestone Jul 10, 2019
Copy link
Member

@adonath adonath left a comment

Thanks @JouvinLea! The changes look good to me, this is exactly what we hand in mind. Note that the old CrabSectrum was called in two tutorial notebooks, that you need to adapt:

  • hess.ipynb
  • spectrum_analysis.ipynb

Open the notebooks with jupyter notebook, make and save the changes and the run gammapy jupyter --src . strip

@JouvinLea
Copy link
Author

@JouvinLea JouvinLea commented Jul 11, 2019

@adonath yes I just realized that those two notebooks were failing!
I made the changes!

@JouvinLea JouvinLea force-pushed the refactor_crab_spectrum branch from 9c91a31 to 3a6cbb4 Jul 11, 2019
@adonath
Copy link
Member

@adonath adonath commented Jul 11, 2019

Thanks @JouvinLea! In your last commit 447ddf0 you accidentally committed the output of the spectrum_analysis.ipynb notebook. Could you please remove this again? Here's how:

git reset HEAD~
gammapy jupyter --src . strip
git commit - "Strip spectrum analysis notebook"
git push -f refactor_crab_spectrum

Then it's ready to be merged...

@JouvinLea JouvinLea force-pushed the refactor_crab_spectrum branch from 447ddf0 to c546544 Jul 11, 2019
@JouvinLea JouvinLea force-pushed the refactor_crab_spectrum branch from 1719c3c to 52ad6be Jul 11, 2019
@adonath adonath merged commit cf1727d into gammapy:master Jul 11, 2019
9 checks passed
@adonath
Copy link
Member

@adonath adonath commented Jul 11, 2019

Thanks a lot @JouvinLea!

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