cjohnson318/geostatsmodels

Python 3 compatibility?

Closed this issue · 6 comments

First of all thanks for the great project! I love how everything is done step-by-step, e.g. at [0]. Now I prefer to use Python 3 - do you have any plans to update the code so that it can run with the newer Python versions as well?

[0] http://nbviewer.jupyter.org/github/cjohnson318/geostatsmodels/blob/master/notebooks/VariogramAnalysis.ipynb

Considering how you're using urllib2, it looks possible to swap to requests.get from the requests package and get compatibility in both versions in that notebook.

Not sure if other incompatibilities lurk in the code, though. I've been able to import & use parts.

I'd always be pretty careful about importing and hoping since division was reworked: 5/2 is evaluated differently on Python 2 and Python 3. As we can't see which variables are floats and which are ints, this is a pretty dangerous thing. There might be some results and they might be close to the mathematically correct one, but there might be some major flaws as well.

Replacing urllib2 with requests is a nice choice but not my main concern.

Sure, I'm very aware of the floordiv problem; I revised pysal into python 2/3 compatible (and then 3-only) code. We had the same issues there throughout, although we also had extensive test coverage, so we could see exactly where we were using an implicit floordiv.

I only commented on the specific point you raised in your issue.

I looked into the floordiv concern before commenting, and it seems to me that every point where division happens (except for maybe this) it's correctly handled by forcing floats (which ensures floatdiv in python 2) or by ensuring that the matrices are float dtyped.

This is not a guarantee, but it seems that @cjohnson318 was aware of this and has future-proofed the code well already.

Thank you for checking the floatdiv issue! When I wrote "the code" I was referring to the whole project, I was not making myself understood well, sorry for that. Btw, thanks for showing me pysal!

I just found the commit which introduced Python 3 compatibility. That makes it seem to work nicely.

@cjohnson318 alright, I guess with the commit this issue should be solved.