Rewrite AnalysisConfig and update Analysis

[Bultako]

Description

This pull request is a follow-up of #2605. It provides a complete reimplementation of the high-level interface using the recently added framework for settings handling based on pydantic

• Analysis and AnalysisConfig classes are now in separated files.
• The previous jsonschema framework dependencies have been removed
• The several template YAML files have been modified following syntax agreed.
• The code in Analysis class and tests has been highly modified refactored.
• Instantiation of AnalysisConfig with settings in a YAML-formatted string is now done with AnalysisConfig.from_yaml(str)
• Instantiation of AnalysisConfig with settings in a YAML file is now done with AnalysisConfig.from_yaml(filename=file.yaml)
• No existing methods have been removed, no new methods added.
• Observation filtering using obbs_ids in a file and time filtering is not implemented.
• Tutorials have been fixed and documentation slightly modified.

[cdeil]

@Bultako - This looks great!

I'll have a closer look and try it out in the next hour.

The test_config_create_from_yaml if failing. Can you reproduce / fix? Or should I have a look?

https://dev.azure.com/gammapy/gammapy/_build/results?buildId=2641&view=logs&jobId=d7d66435-5ae4-561a-5876-2cd4e19b3ce2&taskId=922dfe32-5b90-53bc-18db-0fa0ecbbd97b&lineStart=142&lineEnd=143&colStart=1&colEnd=1

[cdeil]

@Bultako - See two small comments inline.

[cdeil]

@Bultako - This is almost ready to be merged.

Could you please run this

pytest -v gammapy/analysis --cov gammapy.analysis --cov-report html
open htmlcov/index.html

and add 1-2 tests, or delete code that isn't used?

Specifically add 1-2 lines in some test to check the energy validation, and change an existing test to have an exclusion mask and check that this works.

The preference should always be to be as simple as possible and have as little code as possible, so if you're struggling to come up with a test case for something, consider just deleting that option and code. (I think we can still simplify, but probably we should just merge this in in ~ 1 hour, and then continue in future PRs).

[cdeil]

@Bultako - Can you please run this?

make polish
git add gammapy/analysis
git commit -m 'make polish in gammapy/analysis'
git checkout -- .

This runs isort and black formatting, just on the code you changed, and then discards other formatting changes, that would possibly conflict with other PRs.

@adonath @registerrier @Bultako - I think generally this should be the procedure: we try to keep Gammapy master clean so that people can just run make polish before committing. This requires maintainers to run it before merging in PRs for contributors that don't do it. I think that's fine, usually there's anyways other small edits one has to do, e.g. to fix docstring formatting, clean up variable names, ... And if that's not the case, like in the past weeks most of the time, then at least we can do it for code we're touching using the commands above.

[codecov]

Codecov Report

Merging #2621 into master will increase coverage by 0.06%.
The diff coverage is 99.44%.

@@            Coverage Diff             @@
##           master    #2621      +/-   ##
==========================================
+ Coverage    91.5%   91.56%   +0.06%     
==========================================
  Files         141      141              
  Lines       15942    15848      -94     
==========================================
- Hits        14588    14512      -76     
+ Misses       1354     1336      -18

Impacted Files	Coverage Δ
gammapy/analysis/config.py	100% <100%> (+0.82%)	⬆️
gammapy/analysis/__init__.py	100% <100%> (ø)	⬆️
gammapy/analysis/core.py	98.08% <98.73%> (+4.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69ca083...fb08628. Read the comment docs.

[Bultako]

@cdeil
Thanks very much for this review !
I think I've addressed all your comments.
You can make a final polish if you wish or merge :)

[cdeil]

@Bultako - I'll fix https://travis-ci.org/gammapy/gammapy/jobs/618348141#L3061 and do some polishing and then will merge this in shortly.

[cdeil]

@Bultako - Some fixes and cleanup in a3431c8 and fb08628

I'll merge now.

We should discuss & improve at least the following two points tomorrow:

• AnalysisConfig.update is broken, for some of the inputs no state is changed and nothing is returned. You didn't notice in the test because you don't check that something changed. We need to discuss what options to offer (fewer, please), and then fix for those inputs.
• Can we simplify Analysis.set_model, to have fewer cases? It looks like you return False on invalid inputs, so silently nothing happens. I think we should raise a ValueError instead? Maybe we can remove the I/O and only offer one required input to this method?
• Calling config.help("spam") does nothing. I think it should raise an error when passing some section that doesn't exist? And generally it's not clear how a user would figure out what sections are available, it's not mentioned in the docstring. Overall I'd like to get rid of help and _get_doc_sections, but I guess it's used in the docs build, so not easily possible. Maybe investigate switching to https://sphinx-pydantic.readthedocs.io/ ?

And then there's bigger tasks, like improving the config schema, or reviewing and polishing the code in the Analysis class. But this PR leaves it in a much better state than before, improving that is for future PRs.

@Bultako - Overall, I have to say, I'm super happy with this PR. I think it's a big step forward for usability of Gammapy (config tab completion, validation), and also for maintainability (much clearer separation of responsibilities between Analysis and AnalysisConfig, and I find config.py much more readable than the JSON schema, partly because we simplified the schema, partly because I find Python more readable. Muchas gracias, señor!