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

v1 => v2 && Py3 #2

Merged
merged 3 commits into from
Jul 11, 2020
Merged

v1 => v2 && Py3 #2

merged 3 commits into from
Jul 11, 2020

Conversation

bcipolli
Copy link
Collaborator

@bcipolli bcipolli commented Jul 6, 2020

So, two big changes to the world:

  • Py2 is gone. Long live Py3!
  • v1 is gone. v2 FTW!

Did my best. We're using this in production :)

@@ -1255,8 +1255,8 @@
"input": [
"def func(url):\n",
" # Basic function that doesn't do any caching\n",
" import urllib2\n",
" return urllib2.urlopen(url).read()\n",
" from six.moves.urllib import request\n",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

chose to use six and to maintain py2 since, well, what the heck.


kw = {}
if sys.version_info >= (3,):
kw["use_2to3"] = True

setup(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

tsk tsk, trailing whitespace. Sorry about that, had no idea.

from indicators import IndicatorAPI, IndicatorDataset
from climate import ClimateAPI, InstrumentalDataset, ModelledDataset
from wbpy.indicators import IndicatorAPI, IndicatorDataset
from wbpy.climate import ClimateAPI, InstrumentalDataset, ModelledDataset
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

need fully qualified paths to get this working in py3

"""

BASE_URL = "http://api.worldbank.org/"
BASE_URL = "http://api.worldbank.org/v2/"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

le big change for v1 => v2

@@ -18,7 +18,7 @@ class TestFetchFn(unittest.TestCase):
def setUp(self):
# The response contains non-ascii characters, so encoding can be tested
# against different python versions.
self.url = "http://api.worldbank.org/topic?format=json&"
self.url = "http://api.worldbank.org/v2/topic?format=json&"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hidden again over here...

@mattduck
Copy link
Owner

mattduck commented Jul 6, 2020

@bcipolli this is excellent, thanks!

I'll just test that it works and then will look at merging and doing a new PyPI release. I won't be able to get to it today but aiming for this week.

@mattduck
Copy link
Owner

@bcipolli have just had a look at this and it all seems good - tests pass, the example code still works, etc.

Thanks for taking the time to fix it up. I'm gonna merge and then update a few things - the tox versions, sane versions in requirements.txt, add CI, etc. and then will release a new version to PyPI.

I'll give credit in the README that you were responsible for updating this. Will also provide commit access in case you ever need to fix anything.

@mattduck mattduck merged commit 92d9c35 into mattduck:master Jul 11, 2020
@mattduck
Copy link
Owner

@bcipolli I've released a v3.0 to pypi. Meant to do it last weekend but couldn't get to it.

It includes your PR here, and also a change I made to be compatible with pycountry, which has had a couple of breaking changes over the last few years - one to rename alpha fields like alpha2 and alpha3 to alpha_2 and alpha_3, and one change to a get() function to return None instead of raising a KeyError. Wbpy now supports these new versions (and maintains compatibility with the old versions).

I've temporarily dropped support for python2 and I was running into compatibility issues with some dev dependencies. I'm open to adding it back later if there is a need for it.

Let me know if you run into any problems.

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