Add pyyaml as core dependency #1995
Merged
Conversation
Looks good to me, I have no further comments. |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
This PR adds
pyyaml
as a core dependency of Gammapy.In practice this means that we list
pyyaml
ininstall_requires
insetup.py
, and thatpip install gammapy
will always installpyyaml
automatically as a dependency if it isn't already present.We currently mostly use YAML to read ECSV files, as well as for the
gammapy download
functionality. We already use it for model serialisation a bit, and plan to use it more extensively in the future (xref #329). Very likely YAML will be used in the high-level Gammapy interface as config file format (and for model serialisation), so really the vast majority of users will need it. Fermipy does already use it, so this is not adding an extra dependency for them.The changes here are similar to what @adonath did when we made Scipy a core dependency in #1919 - mostly moving imports to the top and removing a few
requires_dependency
decorators on tests. I plan to mention this clearly in the changelog, but in a follow-up PR.