forked from UDST/bayarea_urbansim
-
Notifications
You must be signed in to change notification settings - Fork 11
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
Python 3 wrap-up #117
Merged
Merged
Python 3 wrap-up #117
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I was able to get |
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.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR wraps up the Python 3 compatibility work with some cleanup and additional testing. I've merged all the individual changes into a single staging branch that can run in Python 3.6, and also updated some ancillary files like
setup.py
andrequirements.txt
.My recommendation would be to merge the underlying PRs first — #101, #102, #114, #116 — and then evaluate whether this PR should be merged as-is, or reimplemented to be cleaner.
Dependencies
Running the model in Python 3.6 requires newer versions of several of the dependencies that are pinned in
requirements.txt
(they were frozen in place for the prior PBA, I think):My suggestion here is to maintain multiple lists of requirements -- a general one in
setup.py
that avoids pinning version numbers as much as possible, and then task-specificrequirements.txt
files when you need to guarantee that the environment is identical across many runs.The way this works is that
setup.py
is the official installation script -- if you installbaus
and the dependencies aren’t in place yet, the package manager will automatically get the latest versions of them.requirements.txt
is a stand-alone environment description file (and can be named anything you want). If you feed it to Pip or Conda before installingbaus
, you’ll get the specific versions that are listed.Here's the updated
setup.py
. I renamed the oldrequirements.txt
torequirements-2017.txt
, and added a newrequirements-2019.txt
that lists current versions of all the dependencies.Travis tests
Automated tests run on travis-ci.org whenever code is pushed to Github. Currently it's checking whether code meets the PEP8 style guidelines, and running a couple of small unit tests.
In this PR i cleaned up the Travis script to make it much simpler, while still doing the same things. It also now checks whether
baus
can be installed solely by running the setup script (which didn't work before, because the dependencies were listed somewhere else).Travis is now set up to run its tests in both Python 2.7 and 3.6, so if we add additional logic later on it will be tested on both platforms.
(The tests are failing in Python 2.7 right now for an unrelated reason: UDST/orca#41)
Testing the Python 3 updates
Now that we’re able to run the model in Python 3, we want to confirm that the forecast results are equivalent to running it in Python 2.7.
Using a random seed doesn’t work in this case, because there are so many changes to the underlying code paths -- so i ran the same model three times, twice in Python 2.7 and once in Python 3.6, and compared the results. (Mac, Scenario 4, running to 2015.)
Comparing the total households by jurisdiction in 2015 (
runXX_juris_summaries_2015.csv
:tothh
):So, the effect of shifting to Python 3 is in line with what we'd expect from normal stochasticity.