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

PyPy-ready IPython.lib.pretty, new tox.ini and consistent dict/mapping proxies pretty printing #9827

Merged
merged 10 commits into from Aug 2, 2016

Conversation

@danilobellini
Copy link
Contributor

danilobellini commented Jul 29, 2016

  • Fix #9821 (Python 3 only: add a types.MappingProxyType pretty printer + tests)
  • Fix #9776 (In PyPy, avoid replacing the dict representation formatter/printer by the types.DictProxyType one)
  • Add a new tox.ini, easier to test multiple environments (e.g. tox -e py36,pypy -- lib utils call "iptest lib utils" in CPython 3.6 and PyPy) including matplotlib (but in PyPy)
  • Update .travis.yml to test with PyPy (still known to fail, so it's in allow_failures)
  • Fixed everything needed to ensure that test_pretty.py tests passes in every environment (including PyPy)
  • Fixed pretty printing super(...) objects in PyPy. Previously, it was like:
In [1]: class A(object): pass

In [2]: super(A)
Out[2]: ---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
[...]
--> 634     p.pretty(obj.__self__)
[...]
AttributeError: 'super' object has no attribute '__self__'
  • (Python 2 only) Add consistency to pretty print bogus named classes with "<unknown type>". This can be further enhanced to get the definition name, as it's stored at least in the object __repr__
  • Fixed IPython.lib.pretty._dict_pprinter_factory indentation size to be equal to the prefix size
  • (CPython 2 only) Consistent pretty printing for types.DictProxyType with an extra ctypes-based test to instantiate it directly with the Python API. The pretty printing uses the format used by dict proxies types.DictProxyType.__repr__ in CPython 2.7, printing it as "dict_proxy({...})", but with the sorting and line-breaking pretty printing features.
@danilobellini danilobellini force-pushed the danilobellini:pypy branch from dc91a63 to 348489e Jul 29, 2016
Also fixed the _dict_pprinter_factory indentation step
- Add tests for dict proxies (instances of types.DictProxyType)
- Uses the "dict_proxy({...})" pattern for representation, as happens
  in CPython 2.7 with types.DictProxyType.__repr__
- Used ctypes.pythonapi.PyDictProxy_New to instantiate dict proxies
  inside the tests
- Only for CPython 2.7: in PyPy the types.DictProxyType is dict, and
  in CPython 3 there's no such type, just the new
  types.MappingProxyType in CPython 3.3+
The test_pretty.test_dictproxy now is properly skipped on PyPy
In PyPy there's no __self__ attribute for super(.) objects. As objects
always have a __repr__, the found alternative was to use its curried
argument.

Previously, only the class name was verified as part of the result,
now the tests include a regex.
+1 test passing in PyPy
@danilobellini danilobellini force-pushed the danilobellini:pypy branch from 348489e to 6142c8e Jul 29, 2016
@danilobellini danilobellini force-pushed the danilobellini:pypy branch from aa1bd0c to 7b0b09b Jul 29, 2016
@danilobellini
Copy link
Contributor Author

danilobellini commented Jul 30, 2016

Done. If required, I can rebase/stash/split the commits in this PR while it's just an experimental branch (perhaps to have a separate PR for the MappingProxyType update, or to join the .travis.yml updates into a single commit).

For older PyPy (Python 2.7) versions (e.g. v4.0.1), this PR has more failures, including some in test_pretty.py. I'm using PyPy v5.3.1 (the last stable release) but Travis CI still has the v2.5.0 version, which isn't even the last v2.x version, so I've add a PyPy installation to .travis.yml to ensure the results in Travis CI are the same I get here, or at least as similar as possible.

As of today, the last stable PyPy versions are v2.6.1, v4.0.1 and v5.3.1. The last PyPy3 versions are v2.4.0 (a Python 3.2 that allows u-prefixed unicode strings) and v5.2.0-alpha1 (Python 3.3), but this PR is only about PyPy, not PyPy3.

@Carreau
Copy link
Member

Carreau commented Jul 31, 2016

Thanks a lot for that ! I'm assigning myself to look at it this week ! 🎊

@Carreau Carreau self-assigned this Jul 31, 2016
@minrk
Copy link
Member

minrk commented Aug 1, 2016

That's great, thanks @danilobellini!

@@ -292,6 +292,7 @@ def execfile(fname, glob=None, loc=None, compiler=None):


PY2 = not PY3
PYPY = any(k.startswith("pypy") for k in dir(sys))

This comment has been minimized.

Copy link
@minrk

minrk Aug 1, 2016

Member

I think platform.python_implementation() is a nicer place to look for PyPy

@minrk minrk added this to the 5.1 milestone Aug 1, 2016
@Carreau
Copy link
Member

Carreau commented Aug 1, 2016

Done. If required, I can rebase/stash/split the commits in this PR while it's just an experimental branch (perhaps to have a separate PR for the MappingProxyType update, or to join the .travis.yml updates into a single commit).

No need to do that, the only thing we tend to avoid is to merge master into the pull requests as we have some tooling that assume merge commits are merged PR to autolist list of PRs.

For older PyPy (Python 2.7) versions (e.g. v4.0.1), this PR has more failures, including some in test_pretty.py. I'm using PyPy v5.3.1 (the last stable release) but Travis CI still has the v2.5.0 version, which isn't even the last v2.x version, so I've add a PyPy installation to .travis.yml to ensure the results in Travis CI are the same I get here, or at least as similar as possible.

No problem, many tests are doctest, I think we could just skip them.

I think platform.python_implementation() is a nicer place to look for PyPy

Good point, once the PYPY variable is moved I"m og merging this in, and we can refine in subsequent tests !

Thanks a lot !

@minrk minrk merged commit 99d29c2 into ipython:master Aug 2, 2016
3 checks passed
3 checks passed
codecov/patch 95.00% of diff hit (target 0.00%)
Details
codecov/project 73.57% (+0.02%) compared to bf3b45f
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@minrk
Copy link
Member

minrk commented Aug 2, 2016

Thanks a lot!

@danilobellini
Copy link
Contributor Author

danilobellini commented Aug 2, 2016

You're welcome =)

The test_pretty.py is passing in PyPy 5.3.1, but in PyPy 4.0.1 there are 2 failures. I was trying to fix these, where to test in a specific PyPy version here I was using a prepared docker image: https://hub.docker.com/r/library/pypy/

One of such failures is easy to fix (the collections.defaultdict.__module__ is "_collections", so the deferred printers list just need to include a printer to ("_collections", "defaultdict"). But the other failure would require some deeper changes, as super() objects didn't have the __thisclass__ attribute in older PyPy versions: https://bitbucket.org/pypy/pypy/issues/2251

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.