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

Python 3 support #63

Closed
wants to merge 26 commits into from
Closed

Python 3 support #63

wants to merge 26 commits into from

Conversation

kannibalox
Copy link
Contributor

@kannibalox kannibalox commented Jan 22, 2018

Depends on pyroscope/pyrobase#6

Notes:

  • I didn't auto-add list() to things, it could introduce some subtle bugs but it would be much uglier
  • peak.util.proxies.ProxyTypes was imported wholesale, I checked the license to make sure it was kosher but you might want to double check me on that
  • auvyon didn't seem to be used anywhere and was removed to spare everyone another PR.
  • 2.6 was previously allowed, this change forces either 2.7 or 3.5
  • linting fatally fails midway through due to AttributeError: 'NoneType' object has no attribute 'infer' pylint-dev/pylint#1817
  • update-to-head.sh is broken due to paver -q develop -U trying to install the non-py3 pyrobase

@YenForYang
Copy link

Ah unfortunate that this fails

@kannibalox
Copy link
Contributor Author

It succeeds just fine when pyroscope/pyrobase#6 is installed

@kannibalox kannibalox force-pushed the py3 branch 2 times, most recently from b6dbcb1 to 40aec5e Compare June 6, 2018 21:07
@@ -236,7 +236,7 @@ def do_import(self):
script_text = sys.stdin.read()

with tempfile.NamedTemporaryFile(suffix='.rc', prefix='rtxmlrpc-', delete=False) as handle:
handle.write(script_text)
handle.write(script_text.encode())
Copy link
Owner

Choose a reason for hiding this comment

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

Make 'utf-8' explicit, because Python2 differs.

@Amar1729
Copy link

@kannibalox

To fix some of the current issues, I added:

  • coverage to requirements-dev.txt
  • git+https://github.com/kannibalox/pyrobase.git@py3 to requirements.txt (removed pyrobase>=0.5)

on my local that makes paver test and paver functest work, but installtest is still unreliable since it pretty much tries using python2.

@Amar1729
Copy link

Also, came up with a few fixes to pavement.py and update-to-head.sh that could help in getting paver installtest running properly (diff upload as a patch):

https://gist.github.com/Amar1729/9500b8878f1f90546bf32e1414534553

@kannibalox
Copy link
Contributor Author

I appreciate the update, but most importantly though @pyroscope's source is canonical, I don't want my fork to be hardcoded anywhere.

@kannibalox
Copy link
Contributor Author

Any word on this? It's literally the last library I use that doesn't support py3, and I'm at the point where I no longer use py2 anymore.

@pyroscope
Copy link
Owner

pyrobase first, which is almost done.

@pyroscope pyroscope self-assigned this Feb 18, 2020
@pyroscope pyroscope added the P1 label Feb 18, 2020
@pyroscope
Copy link
Owner

@kannibalox BTW, I decided – unlike with pyrobase – to go immediately Python 3.6+ only after the 1.0 release, which will make things a lot more streamlined in the short and long term.

Please keep this open for visibility and applying relevant parts of your work / patches.

@kannibalox
Copy link
Contributor Author

I missed updating the metadata to reflect it, but I'm using this code on version 3.6.9. Thanks for all the work you've put in, I'll definitely keep this open.

@Amar1729 Amar1729 mentioned this pull request May 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants