Single source Python 2 and 3 #473

pya · 2015-11-08T23:49:11Z

Many new Python users (rightfully) start with Python 3. Therefore, having FiPy support Python 3 out-of-the box would be very valuable.

This version runs all 195 tests in example/test.py with Python 2.7, 3.4, and 3.5 (with the same source). It uses the very good library future (http://python-future.org/). This allows to test all changes immediately against Python 2.7 and Python 3.

Further development can take advantage of future by programming in Python 3 style (think range, zip, map etc.) and still support Python 2.7. In the distance future, when Python 2.7 support is discontinued, only the imports from future need to be removed to get a pure Python 3 version.

…D arrays. `ravel()` produces 1D arrays. In numpy prior to 1.10 `concatenate()` took any number for `axis``. This is not allowed anymore. Droping `axis` works since the default is `0`.

guyer · 2015-11-25T19:19:30Z

First off, thank you very much for this contribution!

There are a few issues I'd like to see addressed before adoption:

numpy 1.10 axis= has nothing to do with 2 to 3 migration and clobbers Updated to NumPy1.10 #472 (as well as Grid3D broken by numpy 1.10 #475 & Remove incorrect axis argument to concatenate #477 that I entered before seeing this pull request).
Changes to trailing whitespace shouldn't be part of this pull request (unless required by Py3k, which I'm unaware of). I'm not opposed to these changes (although I wish git would stop making such a fuss about something that's none of its business), but they presently clutter up this pull request to the point that it's very hard to see the "real" changes.
old_div strikes me as ugly. I think we have a clear enough idea of when floating and when integer division is called for that we should be selectively using / or //. old_div just masks the problem of ambiguous division with a syntax that's hard to read.
In general, there's too much going on in each commit for me to effectively evaluate, particularly when I approve of most of it, but not all. If the futurize command (assuming that's what you used) has any options to control what it converts, I'd like to see individual, atomic commits that each address a single issue: 1 -> True/0 -> False in one, raw_input -> input in another, print in another, and so on.
I'm inherently pretty conservative about making backward-incompatible changes and python-future will require a big change in the minimum version of Python. I've posted a survey to the mailing list to see if this will inconvenience very many of our users (I'm guessing not, but I'd like to be sure).

Addresses #432

pya · 2015-11-26T08:56:58Z

Thanks for your response. All your points are totally valid. Let's see this PR as a proof-of-concept that has some (serious) "procedural" issues.

Sorry for mixing up the NumPy 1.10 issue here. I did not pay enough attention.
I was not aware that there is no option to suppress white space changes in diffs in PRs. This is just a artifact of my editor being setup like this.
The old_div was not intentional but a result of the use of futurize. See next bullet point.
futurize offers 52 different kind of fixes. We might not want/need all of these (see previous bullet point). Applying one fix at a time and commit each one separately sounds like a good idea.
Python 2.4 seems really old. The only place I need to work with Python 2.5 is in embedded environment where I have no way to determine what Python version I can use. I am really happy with conda environments that allow a clean separation between different combinations of Python and library versions on the same machine. Will see what the survey results say.

I would suggest to take this PR as a basis to work from. Basically, don't merged anything into FiPy now but re-do things based on what I've learned. Next steps cold be:

Wait for the survey results. If the minimum Python version moves up to 2.6, we can proceed, otherwise it will be very difficult to achieve out-of-the-box Python 3 support. Is there a deadline for the survey? If not, setting one might speed up answers.
If Python 2.6 becomes the minimum required version we can proceed, addressing the issues above, i.e. after taking out the NumPy version change and the trailing newlines deletes, go one fix at a time and commit each one separately. Simultaneous Python 2.4 and Python 3 support would be rather challenging.
It would be great to have more feedback from you and other core developers along the way. Maybe breaking things down into multiple PRs, even though they would not work with Python 3 yet but all tests pass with Python 2, could be a solution. I am open to suggestions such as some kind of branching scheme or whatever works best according to your experience.

guyer · 2015-11-26T12:27:47Z

OK, great. I think we're on the same page.

If the survey shows a substantial base still tied to Python 2.4 or 2.5, it should still an option to use six. For the record, I don't think that's what the survey will show. Good idea about setting a deadline.
I'm happy to give feedback along the way. We can either do it via incremental pull requests, or I can just subscribe to your repo and make comments as you push to github.

guyer · 2015-12-05T18:24:47Z

The survey results are unambiguous (although I'd like to think we have more than 29 people using FiPy). Please proceed with using python-future to make FiPy work with both Python 2.x and 3.x. Let me know what branch you're working on and I can monitor and comment there.

pya · 2015-12-06T22:00:12Z

Great to hear. I will get started. BTW, I cannot see the survey results. I only get a survey monkey login page.

guyer · 2015-12-07T18:26:39Z

Survey results are now published at https://www.surveymonkey.com/results/SM-2CDMMVQJ/

woodscn · 2016-03-22T21:40:02Z

Any updates on this?

wd15 · 2016-03-23T14:41:04Z

Sorry @woodscn, neither @wd15 nor @guyer have had time to address this, but I hope to do a round of Python development within the next 6 months and this pull request will be top of the agenda unless @guyer doesn't merge it first.

woodscn · 2016-03-23T14:54:08Z

Fair enough. I'll probably end up maintaining a straight "futurize" branch for the time being, then. At least it sounds like backward compatibility is only necessary as far back as 2.6, which makes things much easier.

guyer · 2016-03-23T15:02:19Z

Actually, there's been nothing to address. @pya was going to submit incremental pull requests. Somebody else is welcome to take this on, but this pull request as written will never be merged.

pya · 2016-03-27T21:53:42Z

Thanks for bringing this up again. I was about to start working on this but had a few urgent things on my plate for the last months. I will probably do something here, starting in a week or so.

woodscn · 2016-03-27T22:40:09Z

I've been working on it too, in the meantime. I broke up the process of futurizing the code into a few smaller steps, and I've been making sure that tests pass (in Python 2) as I go. You can check out my branch here: https://github.com/woodscn/fipy/tree/python23v2

Edit: fixed link

guyer · 2016-03-28T14:38:26Z

We completely understand, Mike. Glad to hear you both are willing to tackle this.

achennu · 2018-02-22T16:57:57Z

I am enjoying using this library a lot, only tempered by having to go back to python2. I would be able to find some effort in the coming weeks to help the python 3 port.

Can somebody please explain what's the best place to contribute this effort?

guyer · 2018-03-01T00:26:26Z

@achennu Thanks for offering, but I'm not sure what you mean by "best place to contribute". Do you mean how do you do a pull request? Or how do you do the Py3k conversion? Or ...?

Ultimately, we need a pull request against usnistgov/fipy. That pull request can be forked from @woodscn's effort, or you can start fresh. The items I requested of @pya in this pull request would all still apply.

achennu · 2018-03-30T11:09:30Z

Yes I know how to make PRs. I have ran a 2to3 version before and it mostly ran. While trying to solve equations though, I got errors relating to wrong opcodes. As I didn't know how to solve that, I stopped. I can look into this. Just to clarify: idea would be to move to a common code supporting python 2.7+ and 3.5+.

…

On Thu, Mar 1, 2018, 01:26 Jonathan Guyer ***@***.***> wrote: @achennu <https://github.com/achennu> Thanks for offering, but I'm not sure what you mean by "best place to contribute". Do you mean how do you do a pull request? Or how do you do the Py3k conversion? Or ...? Ultimately, we need a pull request against usnistgov/fipy. That pull request can be forked from @woodscn <https://github.com/woodscn>'s effort, or you can start fresh. The items I requested of @pya <https://github.com/pya> in this pull request would all still apply. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#473 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH7JDE3EZAOTPQ91njycgvd0S45uluXWks5tZ0AzgaJpZM4GeRE0> .

guyer · 2018-03-30T14:09:18Z

As illustrated here, 2to3conversion does work for develop (43bbbd6).

I recommend you use futurize and preferably change one thing in fipy at a time, i.e., I would prefer one commit per fixer.

guyer · 2019-05-10T20:32:49Z

Fixed by #645

pya added 8 commits November 8, 2015 10:07

Modify doctest. Return value is True not 1 for numpy 1.10.1.

5857986

Fix concatenation of numpy arrays. axis=1 does not make sence for 1…

fc44d06

…D arrays. `ravel()` produces 1D arrays. In numpy prior to 1.10 `concatenate()` took any number for `axis``. This is not allowed anymore. Droping `axis` works since the default is `0`.

Automatic conversion with futurize from python-future.org.

e4dd26a

Fix unicode issues.

910b24f

Fix all but one doctest. They pass with Python 2.7 and 3.4.

394e104

Formatting.

b45dd97

Fix last test.

732a4a7

Add requirements.txt.

444e8e0

guyer added the enhancement label Nov 25, 2015

tkphd assigned pya Jan 3, 2018

tkphd requested a review from guyer January 3, 2018 19:01

guyer mentioned this pull request Apr 26, 2019

Migrate to Py3k #645

Merged

11 tasks

guyer closed this May 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Single source Python 2 and 3 #473

Single source Python 2 and 3 #473

pya commented Nov 8, 2015

guyer commented Nov 25, 2015

pya commented Nov 26, 2015

guyer commented Nov 26, 2015

guyer commented Dec 5, 2015

pya commented Dec 6, 2015

guyer commented Dec 7, 2015

woodscn commented Mar 22, 2016

wd15 commented Mar 23, 2016

woodscn commented Mar 23, 2016

guyer commented Mar 23, 2016

pya commented Mar 27, 2016

woodscn commented Mar 27, 2016

guyer commented Mar 28, 2016

achennu commented Feb 22, 2018

guyer commented Mar 1, 2018

achennu commented Mar 30, 2018 via email

guyer commented Mar 30, 2018

guyer commented May 10, 2019

Single source Python 2 and 3 #473

Single source Python 2 and 3 #473

Conversation

pya commented Nov 8, 2015

guyer commented Nov 25, 2015

pya commented Nov 26, 2015

guyer commented Nov 26, 2015

guyer commented Dec 5, 2015

pya commented Dec 6, 2015

guyer commented Dec 7, 2015

woodscn commented Mar 22, 2016

wd15 commented Mar 23, 2016

woodscn commented Mar 23, 2016

guyer commented Mar 23, 2016

pya commented Mar 27, 2016

woodscn commented Mar 27, 2016

guyer commented Mar 28, 2016

achennu commented Feb 22, 2018

guyer commented Mar 1, 2018

achennu commented Mar 30, 2018 via email

guyer commented Mar 30, 2018

guyer commented May 10, 2019