-
-
Notifications
You must be signed in to change notification settings - Fork 122
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
add missing __slots__ to containers and their base classes (#1144) #1147
Conversation
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.
LGTM! Thanks!
Agreed!
Yes, I thought so too during code review. |
I'll create a follow-up issue for the pytest plugin changes There seems to be an issue with |
It seems that for different python versions, # passes in py37, but not py38+
class A:
@staticmethod # noqa: WPS602
def foo():
...
# passes in py38+, but not py37
class A:
@staticmethod
def foo(): # noqa: WPS602
...
# passes on all versions:
class A:
@staticmethod # noqa: WPS602
def foo(): # noqa: WPS602
... |
Codecov Report
@@ Coverage Diff @@
## master #1147 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 79 79
Lines 2338 2415 +77
Branches 153 153
=========================================
+ Hits 2338 2415 +77
Continue to review full report at Codecov.
|
@ariebovenberg yes, this is true 😞 |
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 agree with your WPS issue, that things like __bases__
, __module__
, etc should be allowed.
Thanks a lot for working on this! 🎉
I have made things!
Checklist
CHANGELOG.md
Related issues
__slots__ = ()
missing in many base classes? #1144Open questions
In adding
__slots__
, internal changes were needed to thepytest
plugin. It works now (as discussed in #1144), but I still have a nagging feeling (monkey-patching and mutable global state will do that... 😉 )I would like to suggest a follow-up change to the pytest plugin: that it undoes its patches after each test (like
unittest.mock.patch
does). The advantages are:ReturnsAsserts
. This means it 100% doesn't affect any other tests (i.e. we get more predictable behavior). Currently all tests running after areturns
-enabled test are patched, those before are not. A very nasty thing to debug if it ever messes with anybody's tests 👀ReturnsAsserts
instance.🤔 Of course, we'll have to see what the performance impact is. Perhaps we might need to split the performance-heavy patching into a separate fixture, so it's only activated when needed