Modernize varnames function #27
Conversation
Currently if you instantiate class which defines hook specs the introspector (`pluggy.varnames`) breaks on methods and incorrectly returns the `self` name. I found this by accident. Although it would be odd it's still conceivable that a user might wish to define hook specs on an instance. The real problem seems to be that varnames doesn't actually disregard `self` like the doc string claims.
Delegate to inspect for for the argument spec and remove the need for the `startindex` kwarg. Avoid inspecting raw code and bring the 'self' check assert into this function. This also adds support for hookspecs defined on instances. Resolves pytest-dev#19
| return () | ||
| if startindex is None: | ||
| startindex = int(inspect.ismethod(func)) | ||
| elif not inspect.isroutine(func): # callable object? |
There was a problem hiding this comment.
since what version of python does that method exist?
There was a problem hiding this comment.
never mind, i just verified its in 2.6
|
I also like it -- @tgoodlet did you get a chance to test this against tox, pytest yet? i guess doing develop installs from all of pluggy, pytest, tox and then running the respective test suites would go a long way. Maybe pytest is not so imnportant actually because pluggy's test suite uses it and pytest will in turn use the latest pluggy and pytest is by far the heaviest user of pluggy i guess. So i am merging this but i'd welcome if you watch this repository here in case related bugs turn up later. |
Actually we vendor |
|
On Fri, Nov 11, 2016 at 07:05 -0800, Bruno Oliveira wrote:
good point. |
|
@hpk42 @nicoddemus @RonnyPfannschmidt that's one thing I tried to bring up before (pytest-dev/pytest#1637). I REALLY think in order to promote active development of pluggy @nicoddemus had made the comment in that discussion:
I feel like this only applies to the case where you're running |
As per #19 this provides an update to
pluggy.varnames().I've successfully run the unit tests across all interpreters on arch linux.
I'd like to test this across all dependent projects (
pytest,tox, etc.) as well (and eventually get those tests integrated in a travis job for this project).Note we'll need to move to
inspect.signature()for py3.6...