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

Make libyaml optional #304

Merged
merged 1 commit into from
Nov 20, 2017
Merged

Conversation

dwhswenson
Copy link
Collaborator

Not all of us have pyyaml installed with libyaml support. For those who don't, this uses the old pure python yaml as fallback. The (much faster) CLoader and CDumper are still preferred.

Uses the approach recommended at: https://pyyaml.org/wiki/PyYAMLDocumentation

@Lnaden
Copy link
Contributor

Lnaden commented Nov 14, 2017

How are you installing OpenMMTools such that libyaml does not get installed? I'm pretty sure the conda installation of pyyaml ships complied against libyaml and should have CLoader/Dumper

@dwhswenson
Copy link
Collaborator Author

How are you installing OpenMMTools such that libyaml does not get installed?

pip install -e . I never said I was a normal user. ;-) And before you ask, pyyaml was installed via MacPorts, because I'm a fossil and haven't moved to brew yet. I remain the guinea pig who doesn't use conda on my personal machines (for a little while longer, at least) -- specifically to catch edge cases like this.

Anyway, this is just intended to be a little fix for the oddballs like me. It won't affect most users.

The AppVeyor failure is weird -- this PR doesn't touch anything in the conda recipe or in the appveyor.yml, so I assume that was just bad luck.

@Lnaden
Copy link
Contributor

Lnaden commented Nov 14, 2017

And before you ask, pyyaml was installed via MacPorts

Ah, that explains a fair amount! Good to see someone is still looking for non-standard corner cases (and fixing it!)

The AppVeyor failure is weird

Yeah, AppVeyor is.... touchy. You are right that this PR should not touch AppVeyor. I'll double check to make sure its just an AppVeyor glitch and not something which needs addressed. Otherwise I think this PR is fine and I'll merge it in if I can't find anything odd

@Lnaden Lnaden merged commit 4ba8606 into choderalab:master Nov 20, 2017
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.

2 participants