-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Type inference and hasattr #1424
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
Comments
I ran into an instance of this in the Zulip test runner code, where we add an attribute to a test method via a decorator to track the fact it's expected to be slow, and then check it like this:
See https://github.com/zulip/zulip/blob/master/zerver/lib/test_runner.py for the full code. |
In general i despise hasattr() checks; it's usually much better to add a On Saturday, June 4, 2016, Tim Abbott [email protected] wrote:
--Guido (mobile) |
Just to be clear, these are attributes monkey-added to a function, not a class, so I don't think that solution would work here. |
So there's no place to declare their type anyway. Maybe using three-arg getattr is better? |
Likely has false positives if one of the elements on the union is an ABC |
I was wondering whether the protocol/structural subtyping work could be used on a |
This also affects using features from future python versions (I'm aware that we can do sys.version_info compare but IMHO this case shouldn't give errors) import ast
if hasattr(ast, "unparse"):
print(ast.unparse)
else:
print("no") |
This might be interesting for some people who encounters this problem. You could use the getattr function to get the value and set the default value to unknown = getattr(<object>, 'valname', None)
if unknown is None:
# your action if it's None |
I came across this issue when using multiple inheritance in the context of SQLAlchemy’s Mixins. Take the Mixing in Columns example code which adds a property For both, if hasattr(self, "create_at"):
create_at = self.create_at and try:
create_at = self.create_at
except AttributeError:
# Do something else. mypy reports the For the time being I’ve chosen to |
@jenstroeger Hmm, it sounds like something off, why is the definition of |
Recommendation: python/mypy#1424 (comment) Also ignore 2 mypy errors I don't know how to fix
* fix: improve type hints * fix: initialize message attribute in Error as None Recommendation: python/mypy#1424 (comment) Also ignore 2 mypy errors I don't know how to fix * fix: return None in negative branch to fix mypy error * fix: return None to avoid mypy error and fix type hint * fix: add missing type in handle and improve the typing of certain Result methods. Co-authored-by: acostapazo <[email protected]>
Based on when the fix above was merged, it should be in mypy 0.982, right? It still doesn't seem to be working, e.g. this: class C:
pass
c = C()
setattr(c, "foo", 42)
if hasattr(c, "foo"):
print(c.foo) still leads to |
Any updates on this? When will the fix make it into mypy? Thanks so much! |
@BlackHC if you follow @JelleZijlstra's instructions above you can see that it should be in mypy 0.991 |
Thanks! Upgraded my package dependencies and it works perfectly now! (You made me realize that |
I just encountered code like this:
The type of
x
was an ABC but it doesn't includeinitialize
. This can be easily worked around by rewriting it like this:However, mypy could do better, plausibly. For example:
x
has a union type, infer only union item types with attributeinitialize
after thehasattr
check. So if type ofx
isUnion[str, X]
andX
hasinitialize
, infer type ofx
to beX
in the if body.hasattr
check (a little likeOptional[...]
requiring something like anis not None
check). Not sure what the syntax for this would be like. It would be nice to support these in ABCs as well.The text was updated successfully, but these errors were encountered: