Skip to content
This repository has been archived by the owner on Jan 10, 2023. It is now read-only.

Add support for Python 3 #19

Merged
merged 4 commits into from Nov 21, 2018
Merged

Add support for Python 3 #19

merged 4 commits into from Nov 21, 2018

Conversation

trein
Copy link
Contributor

@trein trein commented Sep 5, 2018

No description provided.

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

@trein
Copy link
Contributor Author

trein commented Sep 5, 2018

@zixia You may need to add a comment to this PR so that @googlebot can confirm you are ok with me publishing a change authored by you.

@huan
Copy link
Contributor

huan commented Sep 6, 2018

@googlebot I'm ok with my commits being contributed to this project from @trein

Copy link
Contributor

@huan huan left a comment

Choose a reason for hiding this comment

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

LGTM

@huan
Copy link
Contributor

huan commented Sep 13, 2018

@trein Thank you very much that the Unite Tests under Python 2.7 seems that they had been fixed.

The Python 3 had not passed yet, I had just fixed an xrange problem in python3 and it seems still have other issues, see: https://travis-ci.com/google/pinject/jobs/145436801

Could you please have a look at them? We will be able to publish the new version to PyPI after we fixed all unit tests.

And please feel free to let me know if we need to change any Travis CI settings.

Cheers!

@googlebot
Copy link

CLAs look good, thanks!

- Use Six package, a simple utilities for wrapping over
  differences between Python 2 and Python 3

- Encapsulate compatibility methods in 'support.py' module

- Introduce unit tests for 'support.py' module
@trein
Copy link
Contributor Author

trein commented Sep 30, 2018

Finally got all tests to pass. Not as easy as it seemed. Please, let me know your thoughts.

@huan
Copy link
Contributor

huan commented Sep 30, 2018

Cheers! Will have a look into it tomorrow.

@alexander-myltsev
Copy link

Please, describe how to install it. Now I had to manually download decorator source code and put it to third_party folder. And manually change from _gdbm import * to from dbm import * at /envs/py36/lib/python3.6/dbm/gnu.py on OSX. Is there an easier way to set it up? Maybe there is a package somewhere?

@rncry
Copy link

rncry commented Nov 19, 2018

@alexander-myltsev in the setup.py file you can remove the unused package dependency on pinject/third_party, which fixes your first problem.

@trein
Copy link
Contributor Author

trein commented Nov 19, 2018

@rncry Good call. I will remove the reference to pinject/third_party in setup.py.

Update: I just noticed that you have a pull request for that. Thanks!

@rncry
Copy link

rncry commented Nov 20, 2018

@alexander-myltsev the second issue is probably due to your version of python not being built with correct support for gdbm/dbm. On mac you can do something like:

$ brew uninstall python3
$ brew install gdbm
$ brew install python3

@rncry
Copy link

rncry commented Nov 20, 2018

@huan how close is this to release?

@huan
Copy link
Contributor

huan commented Nov 20, 2018

Hope I would have time to get this done tomorrow!

@rncry
Copy link

rncry commented Nov 20, 2018

Amazing :) thanks

@huan huan merged commit 1e785b5 into google:master Nov 21, 2018
@rncry
Copy link

rncry commented Nov 21, 2018

@huan just noticed my pr wasn't merged into this one.. setup.py will be broken(!)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants