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

Add GammaCat source catalog #793

Merged
merged 7 commits into from Nov 24, 2016

Conversation

Projects
None yet
2 participants
@adonath
Member

adonath commented Nov 22, 2016

This PR adds a first version of SourceCatalogGammaCat and SourceCatalogObjectGammaCat.

TODO:

  • Add minimal tests for a source with spectral models pl, ecpl and pl2
  • Add a minimal docs example
  • Add further info to .summary()

@cdeil Plotting should already partly work

@adonath adonath added the feature label Nov 22, 2016

@adonath adonath added this to the 0.6 milestone Nov 22, 2016

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Nov 22, 2016

Member

@adonath - Looks good to me. Just merge whenever you like, or ping me again if you want a review.

I see you're introducing GAMMA_CAT as an environment variable here.
That should probably be prominently mentioned in the docs, and the error message on what to do if it's not set or the file isn't there should also be good, with instructions or a link to instructions how to get it.
Maybe for now, adding a section on http://docs.gammapy.org/en/latest/datasets/index.html on how to get gamma-cat and use it with Gammapy would also be useful?

Another thing to do is to add this to .travis.yml in the same way we have gammapy-extra there now.

Member

cdeil commented Nov 22, 2016

@adonath - Looks good to me. Just merge whenever you like, or ping me again if you want a review.

I see you're introducing GAMMA_CAT as an environment variable here.
That should probably be prominently mentioned in the docs, and the error message on what to do if it's not set or the file isn't there should also be good, with instructions or a link to instructions how to get it.
Maybe for now, adding a section on http://docs.gammapy.org/en/latest/datasets/index.html on how to get gamma-cat and use it with Gammapy would also be useful?

Another thing to do is to add this to .travis.yml in the same way we have gammapy-extra there now.

@adonath adonath merged commit 2c75251 into gammapy:master Nov 24, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@adonath adonath deleted the adonath:add_gammacat_catalog_class branch Nov 24, 2016

@cdeil cdeil changed the title from WIP: Add Gammacat catalog and object classes to Add Gammacat catalog and object classes Nov 24, 2016

@cdeil cdeil changed the title from Add Gammacat catalog and object classes to Add GammaCat source catalog Nov 24, 2016

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Nov 24, 2016

Member

@adonath - Thanks!

I quickly tried it out. Some suggestions:

Follow-up PR makes sense, no?
You are me?
(I have a small preference to continue with data entry)

Member

cdeil commented Nov 24, 2016

@adonath - Thanks!

I quickly tried it out. Some suggestions:

Follow-up PR makes sense, no?
You are me?
(I have a small preference to continue with data entry)

@adonath

This comment has been minimized.

Show comment
Hide comment
@adonath

adonath Nov 24, 2016

Member

@cdeil Thanks, for the comments, I'll make a follow up PR.

Member

adonath commented Nov 24, 2016

@cdeil Thanks, for the comments, I'll make a follow up PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment