wemake-python-styleguide
wemake-python-styleguide copied to clipboard
Allow hasattr() function
What's wrong
WPS421 forbids calling some built-in functions, which includes hasattr()
.
How it should be
The function hasattr()
should be allowed. There are certain scenarios where using hasattr()
makes sense, and sometimes is even faster (as even outlined in the referenced video @ 13:34-14:15 mark)
For code readability, if hasattr(foo, 'bar'):
is cleaner than if getattr(foo, 'bar', None):
.
I believe there was an argument against using hasattr()
in python2, but I don't believe the same argument is valid for python3.
Flake8 version and plugins
n/a
pip information
n/a
OS information
n/a
Your link to a video is https://wemake-python-stylegui.de/en/latest/pages/usage/violations/best_practices.html#wemake_python_styleguide.violations.best_practices.WrongFunctionCallViolation
Can you please share a correct one?
Oops, my wording was awful. I meant to say that the video referenced in the See Also documentation for WPS421 ...
The link to the video is https://www.youtube.com/watch?v=YjHsOrOOSuI
I don't mind submitting a PR, but before going through the motions, any thoughts around a yay or nay on this?
I confirm that this no longer produce unexpected result:
class A:
@property
def x(self):
print('wow')
hasattr(A, 'x') # True
x
is not evaluated, when x
is a @property
.
But, when x
is a descriptor it still does this ugly thing:
class GetSet:
def __get__(self, *args):
print('wow')
class A:
x = GetSet()
hasattr(A, 'x') # prints "wow"
Why is that important? Because semantically hasattr
is not very expected to run this code.
But, getattr
has clear semantics: get me this argument and run all the machinery ⚙️ 🔥
So, I am still not completely sold on allowing it.
These are good points. However, getattr
outputs wow
just like hasattr
does. By that logic, getattr
should be added to the FUNCTIONS_BLACKLIST as well then, yes?
To be clear, I don't advocate blacklisting getattr
as there are semantic and performance reasons for using it.
Semantically, I think that if hasattr(A, 'foo')
is clearer than if getattr(A, 'foo', False)
, which was the reasoning behind opening this issue. Do you agree?
what @Dresdn says makes sense, one more reason in favor of allow hasattr
is that the logic behind this one is performed by calling getattr
and catching AttributteError, so, at the end both funcions are base on the same logic, It does not make sense to allow getattr
and and issue a warning on hasattr
I feel that the existing alternative to hasattr
is also prone to mistakes, due to the need for a sentinel (None
).
Using getattr(item, 'x', None)
to see if x
has been defined will be False
when item.x = None
. There isn't another language construct that I know of that can properly tell whether an attr has been defined. And this mistake is more likely, no? It has bitten me before.
In light of this, couldn't hasattr
be allowed while we pursue a fix of the descriptor behavior in the Python interpreter itself?