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

Compat #33

Merged
merged 11 commits into from
Jul 30, 2014
Merged

Compat #33

merged 11 commits into from
Jul 30, 2014

Conversation

cc7768
Copy link
Member

@cc7768 cc7768 commented Jul 30, 2014

All files in the quantecon package should be python 3 compatible now. All files except wb_download.py should be python 3 compatible in the examples directory. The file wb_download.py wasn't as clear how to go about fixing as the others because of changes in the structure of urllib and urllib2. I think that is a minor thing that can be taken care of in the near future though. Submitting PR.

Note: Using this one instead of other PR because I had pulled some of the tests branch and deleted those files which would have caused issues with pulling the tests branch later. Spencer did some git voodoo (cherry-pick) to fix it.

@sglyon
Copy link
Member

sglyon commented Jul 30, 2014

I just pushed one more commit to make examples/wb_download.py work on both versions ;)

import matplotlib.pyplot as plt
from pandas.io.excel import ExcelFile
import urllib

if sys.version_info[0] == 2:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to add these lines to make it compatible with python 2 and python 3.

This obfuscates the example a bit. I think it is probably worth it so that this isn't the only file we have that doesn't run on python 3, but I wanted to get other thoughts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, thanks.

@jstac
Copy link
Contributor

jstac commented Jul 30, 2014

Hey guys. Great work, thank you. Just to clarify, everything runs on both 2 and 3? I thought I'd see more from __future__ import xyz statements at the top of the files...

@sglyon
Copy link
Member

sglyon commented Jul 30, 2014

Ahh, we haven't run through the solutions notebooks yet. I'll do that now and let you know when I have finished. Other than that I am pretty sure everything is working

Also I realize that the line comment I made above doesn't show the context very well. the line I marked was the first of 4 that I added. You may have to go to the diff to see clearly.

@sglyon
Copy link
Member

sglyon commented Jul 30, 2014

Just finished running through all the solutions notebooks. They all work on both languages.

@jstac
Copy link
Contributor

jstac commented Jul 30, 2014

Great work on solutions notebooks, many thanks. It seems the Travis CI build failed --- I'll wait on merging this branch until that's fixed or you let me know otherwise.

@sglyon sglyon mentioned this pull request Jul 30, 2014
scipy.optimize.fmin_slsqp doesn't work the same in python 2 and 3

I switched to using the COBYLA method and it works on both versions.

Also there was an ImportError in odu_plot_densities
@sglyon
Copy link
Member

sglyon commented Jul 30, 2014

We have now confirmed that all the programs in the examples, solutions, and quantecon directories work on both python 2 and 3.

I think this is ready to close.

John, don't worry about travis messages right now. We need to wait until we get the tests branch merged before those messages will be totally useful.

jstac added a commit that referenced this pull request Jul 30, 2014
@jstac jstac merged commit 74551f0 into master Jul 30, 2014
@jstac jstac deleted the compat branch July 30, 2014 22:24
@jstac
Copy link
Contributor

jstac commented Jul 30, 2014

Great work, many thanks.

jstac added a commit that referenced this pull request Aug 25, 2014
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.

3 participants