Handle descriptors defined in PyProxyBase subclasses like C #6

jamadden · 2015-05-07T15:27:21Z

This fixes #5 plus a few other discrepancies I noticed during testing.

It also fixes the tox py27-pure environment, which wasn't actually exercising the pure Python code if the .so had already been built; now it takes an approach similar to other libraries and lets the environment variable take precedence.

… C version does. This fixes #5.

…rridable.

jamadden · 2015-05-07T16:13:48Z

Hmm, it looks like zope.container._proxy might have similar issues.

jamadden · 2015-05-07T16:21:30Z

Please hold. I just discovered an issue with getting methods with getattr that was not covered by the test cases.

jamadden · 2015-05-07T16:31:35Z

Ok, that last commit did it. Unit tests pass here as they do in the subset of the application I was working with that prompted this discovery in the first place.

…is will let zope.container._proxy reuse all the fixes in this branch.

jamadden · 2015-05-08T18:17:13Z

I have a branch for zope.container that solves any issues it had with descriptors by making it use the PyProxyBase code from here. It's nice because it removes a lot of code :) Locally all the tests pass for me, including the ZODB tests, but it needs zopefoundation/persistent#20 to work for anyone else.

tseaver · 2015-05-19T17:22:38Z

This change now fails to cover the pure-python __complex__:

$ git stat
# On branch master
nothing to commit (working directory clean)
$ tox -e coverage
GLOB sdist-make: /home/tseaver/projects/Zope/Z3/zopetoolkit/src/zope.proxy/setup.py
coverage inst-nodeps: /home/tseaver/projects/Zope/Z3/zopetoolkit/src/zope.proxy/.tox/dist/zope.proxy-4.1.5.dev0.zip
coverage runtests: PYTHONHASHSEED='3947475143'
coverage runtests: commands[0] | pip uninstall -y zope.proxy
Uninstalling zope.proxy-4.1.5.dev0:
  Successfully uninstalled zope.proxy-4.1.5.dev0
coverage runtests: commands[1] | pip install -e .
Obtaining file:///home/tseaver/projects/Zope/Z3/zopetoolkit/src/zope.proxy
Requirement already satisfied (use --upgrade to upgrade): zope.interface in ./.tox/coverage/lib/python2.6/site-packages (from zope.proxy==4.1.5.dev0)
Requirement already satisfied (use --upgrade to upgrade): setuptools in ./.tox/coverage/lib/python2.6/site-packages (from zope.proxy==4.1.5.dev0)
Installing collected packages: zope.proxy
  Running setup.py develop for zope.proxy
Successfully installed zope.proxy
coverage runtests: commands[2] | nosetests --with-xunit --with-xcoverage
.....................................................................................................................................................
Name                    Stmts   Miss  Cover   Missing
-----------------------------------------------------
zope.proxy                271      0   100%   
zope.proxy._compat          2      0   100%   
zope.proxy.decorator       18      0   100%   
zope.proxy.interfaces      10      0   100%   
-----------------------------------------------------
TOTAL                     301      0   100%   
----------------------------------------------------------------------
Ran 149 tests in 0.164s

OK
___________________________________ summary ____________________________________
  coverage: commands succeeded
  congratulations :)
$ git checkout NextThought-python-provided 
Switched to branch 'NextThought-python-provided'
$ tox -e coverage
GLOB sdist-make: /home/tseaver/projects/Zope/Z3/zopetoolkit/src/zope.proxy/setup.py
coverage inst-nodeps: /home/tseaver/projects/Zope/Z3/zopetoolkit/src/zope.proxy/.tox/dist/zope.proxy-4.1.5.dev0.zip
coverage runtests: PYTHONHASHSEED='4040238471'
coverage runtests: commands[0] | pip uninstall -y zope.proxy
Uninstalling zope.proxy-4.1.5.dev0:
  Successfully uninstalled zope.proxy-4.1.5.dev0
coverage runtests: commands[1] | pip install -e .
Obtaining file:///home/tseaver/projects/Zope/Z3/zopetoolkit/src/zope.proxy
Requirement already satisfied (use --upgrade to upgrade): zope.interface in ./.tox/coverage/lib/python2.6/site-packages (from zope.proxy==4.1.5.dev0)
Requirement already satisfied (use --upgrade to upgrade): setuptools in ./.tox/coverage/lib/python2.6/site-packages (from zope.proxy==4.1.5.dev0)
Installing collected packages: zope.proxy
  Running setup.py develop for zope.proxy
Successfully installed zope.proxy
coverage runtests: commands[2] | nosetests --with-xunit --with-xcoverage
................................................................................................................................................................
Name                    Stmts   Miss  Cover   Missing
-----------------------------------------------------
zope.proxy                293      1    99%   244
zope.proxy._compat          2      0   100%   
zope.proxy.decorator       18      0   100%   
zope.proxy.interfaces      10      0   100%   
-----------------------------------------------------
TOTAL                     323      1    99%   
----------------------------------------------------------------------
Ran 160 tests in 0.163s

OK
___________________________________ summary ____________________________________
  coverage: commands succeeded
  congratulations :)

jamadden · 2015-05-19T17:35:50Z

Hmm. That's incredibly odd. I'll see if I can figure out why that is.

jamadden · 2015-05-19T18:10:34Z

@tseaver That was interesting. It turns out that if you switch the coverage environment to python2.7 instead of python2.6, you get 100% coverage again:

$  cat tox.ini | grep -A2 testenv:coverage
[testenv:coverage]
basepython =
    python2.7
$ tox -e coverage
GLOB sdist-make: /zope.proxy/setup.py
coverage create: //zope.proxy/.tox/coverage
coverage installdeps: nose, coverage, nosexcover
coverage inst: //zope.proxy/.tox/dist/zope.proxy-4.1.5.dev0.zip
coverage installed: -f file:////buildout-eggs,coverage==3.7.1,nose==1.3.6,nosexcover==1.0.10,zope.interface==4.1.2,zope.proxy==4.1.5.dev0
coverage runtests: PYTHONHASHSEED='3256792812'
coverage runtests: commands[0] | pip uninstall -y zope.proxy
Uninstalling zope.proxy-4.1.5.dev0:
  Successfully uninstalled zope.proxy-4.1.5.dev0
coverage runtests: commands[1] | pip install -e .
Obtaining file:////zope.proxy
Requirement already satisfied (use --upgrade to upgrade): zope.interface in ./.tox/coverage/lib/python2.7/site-packages (from zope.proxy==4.1.5.dev0)
Requirement already satisfied (use --upgrade to upgrade): setuptools in ./.tox/coverage/lib/python2.7/site-packages (from zope.proxy==4.1.5.dev0)
Installing collected packages: zope.proxy
  Running setup.py develop for zope.proxy
Successfully installed zope.proxy
coverage runtests: commands[2] | nosetests --with-xunit --with-xcoverage
................................................................................................................................................................
Name                    Stmts   Miss  Cover   Missing
-----------------------------------------------------
zope.proxy                293      0   100%
zope.proxy._compat          2      0   100%
zope.proxy.decorator       18      0   100%
zope.proxy.interfaces      10      0   100%
-----------------------------------------------------
TOTAL                     323      0   100%
----------------------------------------------------------------------
Ran 160 tests in 0.254s

OK

Under Python 2.7, Modules/complexobject.c (where complex() is implemented in the function complex_new) uses PyObject_LookupSpecial to find the __complex__ method on the type of the object (as expected, and as done for things like __int__ and such).

However, under Python 2.6, that same code path simply does a PyObject_GetAttr, i.e., it doesn't treat __complex__ as a special method. It even has the comment /* XXX Hack to support classes with __complex__ method */. Therefore, under Python 2.6 PyProxyBase.__getattribute__ is invoked, which does correctly do a descriptor lookup on the (subclasses of the) type and finds nothing, passing it on to the underlying object. The behaviour remains the same, though.

Can we just update the coverage environment?

tseaver · 2015-05-19T18:35:18Z

Weird. I guess it would be fine to update the basepython for [testenv:coverage] to python2.7.

Python 2.6 uses a "hack" to call __complex__ and so we lost coverage of that. 2.7 does it correctly. See #6 (comment)

jamadden · 2015-05-19T18:54:12Z

Ok, done.

Handle descriptors defined in PyProxyBase subclasses like C

jamadden added 2 commits May 7, 2015 10:23

Handle descriptors defined in PyProxyBase subclasses the same way the…

fd0e8f7

… C version does. This fixes #5.

zope.container._proxy relied on the implementation detail of PyNonOve…

a193275

…rridable.

Fix getting methods as attributes.

9e244f2

Make it easier to subclass PyProxyBase using multiple-inheritance. Th…

dbf86b2

…is will let zope.container._proxy reuse all the fixes in this branch.

jamadden mentioned this pull request May 14, 2015

Support for ZODB on PyPy zopefoundation/persistent#20

Merged

Update the tox coverage environment to python2.7.

7d0d502

Python 2.6 uses a "hack" to call __complex__ and so we lost coverage of that. 2.7 does it correctly. See #6 (comment)

tseaver added a commit that referenced this pull request May 19, 2015

Merge pull request #6 from NextThought/python-provided

37e9ea6

Handle descriptors defined in PyProxyBase subclasses like C

tseaver merged commit 37e9ea6 into zopefoundation:master May 19, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle descriptors defined in PyProxyBase subclasses like C #6

Handle descriptors defined in PyProxyBase subclasses like C #6

jamadden commented May 7, 2015

jamadden commented May 7, 2015

jamadden commented May 7, 2015

jamadden commented May 7, 2015

jamadden commented May 8, 2015

tseaver commented May 19, 2015

jamadden commented May 19, 2015

jamadden commented May 19, 2015

tseaver commented May 19, 2015

jamadden commented May 19, 2015

Handle descriptors defined in PyProxyBase subclasses like C #6

Handle descriptors defined in PyProxyBase subclasses like C #6

Conversation

jamadden commented May 7, 2015

jamadden commented May 7, 2015

jamadden commented May 7, 2015

jamadden commented May 7, 2015

jamadden commented May 8, 2015

tseaver commented May 19, 2015

jamadden commented May 19, 2015

jamadden commented May 19, 2015

tseaver commented May 19, 2015

jamadden commented May 19, 2015