allenai / deep_qa

A deep NLP library, based on Keras / tf, focused on question answering (but useful for other NLP too)
Apache License 2.0
404 stars 133 forks source link

WIP: DO NOT MERGE! #384

Closed schmmd closed 7 years ago

schmmd commented 7 years ago

This PR is to test the build in Travis--DO NOT MERGE.

matt-gardner commented 7 years ago

https://stackoverflow.com/a/39162893/1467533

Try adding

env = PYTHONHASHSEED=2157

to pytest.ini. That should make the tests pass even with the change you made in your previous commit (and would be an improvement!).

schmmd commented 7 years ago

@matt-gardner no dice. I think it sets the environment variable after python has been loaded, so PYTHONHASHCODE does not take effect. Yikes, what a setup!

matt-peters commented 7 years ago

FWIW, you can specify environmental variables in .travis.yml: https://docs.travis-ci.com/user/environment-variables/

matt-gardner commented 7 years ago

@matt-peters, that's a good idea.

schmmd commented 7 years ago

Yes, that's probably better than https://github.com/allenai/deep_qa/blob/master/build_tools/travis/test_script.sh#L13. I'll try it out today if I have time.

matt-gardner commented 7 years ago

Oh, wait, yeah, we already do that, and that's why it works. Not sure that setting it in .travis.yml actually buys us anything. CI passes; the issue is that it's opaque and you can easily miss things if you're setting up the environment for yourself, not in CI. Having a failing test is a good idea here, but I'm not sure that moving it from where it is in the script into .travis.yml helps with clarity.