search

santoshphilip / eppy

scripting language for E+, Energyplus
MIT License
158 stars 66 forks source link

Python3 transition - alternate strategy #87

Closed santoshphilip closed 8 years ago

santoshphilip commented 8 years ago

Right now we are using 2to3 to transition from python2 to python3. So we have

Another option is too use the package six From six's documentation: "Six provides simple utilities for wrapping over differences between Python 2 and Python 3. It is intended to support codebases that work on both Python 2 and 3 without modification".

Essentially this allows to write and maintain only one code base, that will run in python2 and python3. @digwanderlust thinks this is a viable and preferred option. I am inclined to trust his judgement on this

The advantage is using six is that:

@eayoungs , If we go down the six route, issue #64 becomes redundant. Sorry to muddy the situation like this.

I suggest, we proceed in the following manner:

jamiebull1 commented 8 years ago

Thanks, @digwanderlust - that was great advice. I decided to give this a try today and it took just over an hour to get everything working with six. I'll push to this repo so @santoshphilip can have a look when he's next around but for my money, this is definitely the way to go. @eayoungs , feel free to chime in if there's anything you think this approach misses.

santoshphilip commented 8 years ago

wohoooo ! This is cool.I am on a flaky internet connection right now. So I'll take a look at later.

  From: Jamie Bull <notifications@github.com>

To: santoshphilip/eppy eppy@noreply.github.com Cc: santoshphilip santosh_philip@yahoo.com; Mention mention@noreply.github.com Sent: Friday, June 3, 2016 9:30 AM Subject: Re: [santoshphilip/eppy] Python3 transition - alternate strategy (#87)

Thanks, @digwanderlust - that was great advice. I decided to give this a try today and it took just over an hour to get everything working with six. I'll push to this repo so @santoshphilip can have a look when he's next around but for my money, this is definitely the way to go. @eayoungs , feel free to chime in if there's anything you think this approach misses.— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

digwanderlust commented 8 years ago

No problem, glad you were able to get everything working with six so easily.

santoshphilip commented 8 years ago

wow ! I took a look at this. It was like magic :-)

I think this is the way to go.

santoshphilip commented 8 years ago

@jamiebull1 Shall we pause other work and move to six ?

jamiebull1 commented 8 years ago

Hell yeah!

santoshphilip commented 8 years ago

Heh !

may be call that branch six_finally :-) If all goes well we can merge it back to master and do a release.

Can you go ahead and make a start on it.

jamiebull1 commented 8 years ago

@santoshphilip - I've just brought the existing #100 six PR up to date with develop.

santoshphilip commented 8 years ago

@jamiebull1 I would like to understand and review the work you have done in i87_six what would be the best pathway to do that ?

I am going to the Ashrae simbuild in Salt Lake City next week. Might have some time while I am there to work on this.

jamiebull1 commented 8 years ago

Yes, the diff will tell you a lot. But I'll dig out the resources I used when I'm back at my laptop (it's pool o'clock here in Spain).

It's mainly about using idioms that work in either 2 or 3 where possible, then using six to fill in the gaps where there isn't a cross-version idiom.

On Thu, 4 Aug 2016, 17:32 santoshphilip, notifications@github.com wrote:

@jamiebull1 https://github.com/jamiebull1 I would like to understand and review the work you have done in i87_six what would be the best pathway to do that ?

  • Can you point me to the links that I can read up on about six
  • Should I do a diff between develop and i87_six as a way in.
  • is there a good starting point to do this ?

I am going to the Ashrae simbuild in Salt Lake City next week. Might have some time while I am there to work on this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/santoshphilip/eppy/issues/87#issuecomment-237590339, or mute the thread https://github.com/notifications/unsubscribe-auth/ADo-yBqn4PuVkeMT1OApoc4ucEHT_qj6ks5qcgYFgaJpZM4HeyJ4 .

jamiebull1 commented 8 years ago

Ok, so the process I followed is listed below. The most relevant steps are 5 and 9 where the bulk of the changes for cross-version compatibility were made. Most of the rest was to make the CI work. In going through the changes just now I've spotted that pip install eppy won't work cross platform with environment markers, so there's a new commit where that's fixed that too.

  1. Use environment markers to install version-specific packages when installing, e.g:

pydot>1.0; python_version <= '2.7' pydot3k; python_version >= '3.0'

  1. Removed most of p3 directory and adjusted CI specifications to stop changing to the p3 directory when running under python 3.
  2. Added six to requirements.txt
  3. Removed the install_requires section from setup.py since everything is in requirements.txt
  4. Run tests under Python 3, fixing failures as they happen. Diff between the following commits to see details of what changed. There's a good list of the features/idioms available for cross-version compatibility including using six here:

    9b69d0

    b971bc

    afa6c0e

    931522

  5. Once all tests were passing, removed the rest of the p3 directory and 2to3.sh files
  6. Fixed one more failing test in commit #d15205
  7. A bunch of changes related to CI and coverage testing
  8. Ran 2to3 on the whole codebase to catch any issues in areas that aren't yet exercised by unit tests (I could probably have done this first and saved a lot of time!)
  9. A few more CI-related changes
  10. Fixed setup.py to clean up the things from item no. 4 in this list and allow pip install eppy to work properly for Python 2 and 3
santoshphilip commented 8 years ago

@jamiebull1 ,

I have some crude integration tests written to test writing files to the disk. You can run them on a unix machine by sh integration.sh (you might have to look inside integration.sh to see how to run them on a windows machine)

These tests are failing python3 in branch i87_six when testing on a mac, they fail when writing a window line endings file to disk. it writes it with \n (unix) line endings. You may get a different failure on the windows platform.

This is the only holdup in merging this branch into master. Everything else looks good.

jamiebull1 commented 8 years ago

Great, I'll tidy that up then. That was also something I was concerned about on the improve_coverage branch so I'll add the integration tests there too.

On Tue, 6 Sep 2016, 04:23 santoshphilip, notifications@github.com wrote:

@jamiebull1 https://github.com/jamiebull1 ,

I have some crude integration tests written to test writing files to the disk. You can run them on a unix machine by sh integration.sh (you might have to look inside integration.sh to see how to run them on a windows machine)

These tests are failing python3 in branch i87_six when testing on a mac, they fail when writing a window line endings file to disk. it writes it with \n (unix) line endings. You may get a different failure on the windows platform.

This is the only holdup in merging this branch into master. Everything else looks good.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/santoshphilip/eppy/issues/87#issuecomment-244835146, or mute the thread https://github.com/notifications/unsubscribe-auth/ADo-yH2gXe0XDbE-q32Er2e139C41J-lks5qnM6mgaJpZM4HeyJ4 .

santoshphilip commented 8 years ago

I may be mistaken on why it is failing. In any case it is failing.

jamiebull1 commented 8 years ago

No problem. I'm on it now.

jamiebull1 commented 8 years ago

Resolved in #100