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

Add test files for high level interface #13

Closed
wants to merge 1 commit into from
Closed

Add test files for high level interface #13

wants to merge 1 commit into from

Conversation

Bultako
Copy link
Member

@Bultako Bultako commented May 28, 2021

This PR adds two simple files to test the use of $GAMMAPY_DATA in config variables of the high level interface, as it is described in gammapy/gammapy#3377

@adonath
Copy link
Member

adonath commented May 28, 2021

Thanks, @Bultako! Is adding the data here really needed? The exclusion mask is already here: https://github.com/gammapy/gammapy-data/blob/master/joint-crab/exclusion/exclusion_mask_crab.fits.gz

And the list of observations ids could be just generated in the test with a temporary file? I think adding a file for the two obs id's is not really worth it?

@Bultako
Copy link
Member Author

Bultako commented May 28, 2021

@adonath

Ok for the exclusion mask, but I think the observation list file is needed. If we want to fully test the PR gammapy/gammapy#3377 I guess we need an observation list file in this GAMMAPY_DATA repo. Or I misunderstood gammapy/gammapy#3377 (review)

@adonath
Copy link
Member

adonath commented May 28, 2021

@Bultako Sorry my bad...in the case of obs_file I just meant adding a test at all. It doesn't have to use $GAMMAPY_DATA. I think we can be sure make_path() works, as we use it in many places, but from a quick look it seemed the that the obs_file option is not tested at all neither in test_analysis.py nor test_config.py.

@Bultako
Copy link
Member Author

Bultako commented May 31, 2021

ok
I continue with the discussion in gammapy/gammapy#3377 and I will close this PR if no action is needed here to improve the test coverage.

@Bultako
Copy link
Member Author

Bultako commented May 31, 2021

Closing since it is not needed in gammapy/gammapy#337

@Bultako Bultako closed this May 31, 2021
@adonath adonath deleted the hli branch October 7, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants