Add match_args support to attr.s()
#12111
Conversation
|
CC @Tinche, you might be interested! I would appreciate your review 🙂 |
mypy/plugins/attrs.py
Outdated
| cls=ctx.cls, | ||
| name='__match_args__', | ||
| typ=tuple_type, | ||
| no_serialize=True, |
There was a problem hiding this comment.
Should we serialize this? 🤔
There was a problem hiding this comment.
Yes, probably we should.
|
Phew, excellent. I was worried I'd have to do this myself but I already have a bunch of other projects on my plate. Can give it a review in the next couple of days. |
mypy/plugins/attrs.py
Outdated
| @@ -290,6 +292,12 @@ def attr_class_maker_callback(ctx: 'mypy.plugin.ClassDefContext', | |||
| if kw_only: | |||
| ctx.api.fail(KW_ONLY_PYTHON_2_UNSUPPORTED, ctx.reason) | |||
| return | |||
| if ctx.api.options.python_version[:2] < (3, 10): | |||
| if match_args: | |||
| ctx.api.fail("match_args is not supported before Python 3.10", info.defn) | |||
There was a problem hiding this comment.
Is this what attrs does at runtime? It's a bit strange to me because it seems harmless to set it before 3.10. For example, you might use it in a library if you want 3.10 users to be able to use match, but still support older versions too.
There was a problem hiding this comment.
Yeah looks like we set it conditionally, presumably to avoid a little work and since it'd be useless.
Since the check is in runtime and based on the running interpreter, I'm not sure I understand your concern. Can you elaborate? We can relax this if needed.
There was a problem hiding this comment.
The scenario I'm thinking of is:
- I'm writing a library that supports both 3.9 and 3.10
- The library exposes an attrs class in its API
- I want 3.10 users to be able to use the class with
match
In that case, I'd want to be able to just set match_args=True.
It looks like that's already how the runtime works (it just doesn't set the attribute on 3.9 or lower), but the code in this PR would make mypy emit an error if it's checking in 3.9 mode and sees match_args=True.
There was a problem hiding this comment.
Ah, right, I see. It's true that the parameter exists on earlier Pythons, so it's not an error to use it there. And you can't use __match_args__ on earlier Pythons anyway (I guess you could manually do something with it, but who does that).
Also, match_args defaults to True.
So this check can essentially be removed?
|
Thank you, @Tinche, very helpful. Having someone with domain knowledge is amazing, many details were wrong 👍 |
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
This is a very basic implementation. I am not yet very familiar with
__match_args__.Are there any corner cases that I am missing?