-
-
Notifications
You must be signed in to change notification settings - Fork 147
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
Fix for python 3.6a #64
Conversation
1.3 | ||
--- | ||
|
||
* Fix error when running ``pytest-mock`` in Python 3.6a. Thanks `@hackebrot`_ for the report (`#59`_). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also applies to 3.6.0b1, I suppose?!
Maybe just say 3.6 then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, specially because it is now part of CI. Will change it.
1 similar comment
cd2cc7a
to
1eb1f9f
Compare
Nice one @nicoddemus @blueyed! 👍 |
@@ -10,7 +10,7 @@ | |||
version = '1.2' | |||
|
|||
|
|||
class MockFixture(object): | |||
class MockFixture: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't make this MockFixture
an old-style class on Python 2? Is that intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right. I tried that first as a fix and forgot to revert it.
On the other hand being an old-style class doesn't really matter for MockFixture
because it doesn't have any comparison operators or meta-classes, which is where old-style and new-style classes differ. Do you rather me revert that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer that, as I have no idea how an old-style class works, and I'm sure I'm not the only one 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But apparently @nicoddemus knows how they work and that it does not matter in this case.
Makes the code cleaner, too - so I am +1 for keeping it like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is good btw to forget about old-style things.. ;)
We have tests for catching that hopefully.. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except that maybe @nicoddemus isn't the only one working on that code 😉
And I agree about forgetting about old-style things, that's why it should be a new-style class 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@The-Compiler
Well:
% git shortlog -sn
122 Bruno Oliveira
4 Rafael Bertoldi (fogo)
3 Jurko Gospodnetić
2 Florian Bruhin
2 Pavel Savchenko
2 Tiago Nobrega
2 jhermann
;)
But it's under the pytest-dev umbrella of course, so you are right in that regard.
new-style classes however are only relevant for py2, and I was disregarding this by ignoring old-style things.. ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since @The-Compiler prefers to bring it back, no worries! Pushed 931785c. 😁
This was removed by accident when working on #64
Fix #59