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

pyyaml version pinning leads to conflic with fermipy #131

Closed
henrikef opened this issue Oct 7, 2020 · 11 comments · Fixed by #132
Closed

pyyaml version pinning leads to conflic with fermipy #131

henrikef opened this issue Oct 7, 2020 · 11 comments · Fixed by #132
Labels
dependencies Pull requests that update a dependency file

Comments

@henrikef
Copy link
Contributor

henrikef commented Oct 7, 2020

We require pyyaml version 3.13:

PyYAML==3.13

See #97 for why things break with newer versions of pyyaml.

Unfortunately, fermipy requires gammapy, which has required pyyaml >=5.1 for about a year now (and I believe that newer versions of fermipy are incompatible with older gammapy versions).

So, I believe it's time to try again to get astromodels working with newer versions of pyyaml. Has anyone looked into this at all?

@henrikef henrikef added the dependencies Pull requests that update a dependency file label Oct 7, 2020
@ndilalla
Copy link
Contributor

ndilalla commented Oct 7, 2020

Hi @henrikef! Yes, I am working on this on the development branch for the new fermitools and root6. Astromodels is ready to work with newer pyyaml while in threeML I am still having some issues with the YAML loader that is deprecated in pyyaml 5.1. So we will need to change a bit our code according to this:
https://github.com/yaml/pyyaml/wiki/PyYAML-yaml.load(input)-Deprecation

@henrikef
Copy link
Contributor Author

henrikef commented Oct 7, 2020

Awesome! Let me know if you need any help or would like me to test anything.

@grburgess
Copy link
Contributor

I noticed when I tried to modify to the forward version pyyaml that the safe loader was not compatible with our model structure. Is that still the case?

@henrikef
Copy link
Contributor Author

What about the "FullLoader" or "UnsafeLoader" though?

@grburgess
Copy link
Contributor

@henrikef I think I tried that.. but I cannot remember. @ndilalla how did you get it to work?

@ndilalla
Copy link
Contributor

@henrikef I think I tried that.. but I cannot remember. @ndilalla how did you get it to work?

Locally I used FullLoader and everything seems to work. Unfortunately it breaks the compatibility with pyyaml 3.13. If we want to be still compliant we will have to do something smarter (like checking the pyyaml major version first). What do you think about it?

@henrikef
Copy link
Contributor Author

henrikef commented Oct 13, 2020

Is there a problem requiring a newer version of pyyaml? I vote for no backward compatibility.

@grburgess
Copy link
Contributor

grburgess commented Oct 13, 2020 via email

@ndilalla
Copy link
Contributor

No, no problems. We just need to require pyyaml >= 5.1 in the dependency list. I'll go for it.

@ndilalla
Copy link
Contributor

ndilalla commented Oct 13, 2020

I don't know why the stupid conda continues to put pyyaml==3.13 as requirement even if I've put pyyaml>=5.1 in the meta and setup files! This is crazy...

@ndilalla
Copy link
Contributor

Okay, it should be fixed now.

@ndilalla ndilalla linked a pull request Oct 15, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants