Typechecking attrs-generated classes

attrs removes object boilerplate:

>>> import attr
>>> @attr.s
... class C(object):
...     x = attr.ib(default=42)

This has many advantages, like eq/hash behavior working, reprs working, easy to add runtime validators, &c. However, it's not obvious to me how I can typecheck creating instances of this, because there's no __init__. (I'm using Python 2.7, so comment syntax, if it matters :))

Hi Laurens, this looks like something that should be doable with the (as yet hypothetical) plugin architecture: #1240

A bit more detail, as the problem here isn't just that you can't validate attrs class initializer usage using mypy (and, in fact, you can validate types at runtime using attrs), but also that mypy is emitting false positives, which makes mypy harder to use if you use attrs.

Here's a script thing.py:

from pathlib import Path

from attr import attrib, attrs
from attr.validators import instance_of


@attrs()
class ThingWithPath(object):
    _path = attrib(validator=instance_of(Path))

    def exists(self):
        return self._path.exists()


thing1 = ThingWithPath(Path("."))
print("thing1 exists:", thing1.exists())

thing2 = ThingWithPath(path=Path("."))
print("thing2 exists:", thing2.exists())

Which runs thusly (note the runtime type validation, if that's a thing you like):

wsanchez$ python thing.py
thing1 exists: True
thing2 exists: True
Traceback (most recent call last):
  File "thing.py", line 21, in <module>
    thing3 = ThingWithPath(".")
  File "<attrs generated init 72ca13b0f10a00b12fb26accfd84d196fc2af0c6>", line 5, in __init__
  File "/Users/wsanchez/Dropbox/Developer/PIE/wsanchez/ConfigService/.tox/py35-coverage/lib/python3.5/site-packages/attr/validators.py", line 24, in __call__
    attr, self.type, value,
TypeError: ("'_path' must be <class 'pathlib.Path'> (got '.' that is a <class 'str'>).", Attribute(name='_path', default=NOTHING, validator=<instance_of validator for type <class 'pathlib.Path'>>, repr=True, cmp=True, hash=True, init=True, convert=None, metadata=mappingproxy({})), <class 'pathlib.Path'>, '.')

Running mypy on this script emits:

thing.py:15: error: Too many arguments for "ThingWithPath"
thing.py:18: error: Unexpected keyword argument "path" for "ThingWithPath"
thing.py:21: error: Too many arguments for "ThingWithPath"

And mypy of course exists with a non-zero status.

It's unfortunate, but not unique to attrs -- this is a general problem when you have a metaclass or a class decorator that affects the API of a class. I suppose we could teach mypy to special-case attrs, but that just moves the problem to every other library engaging in such tricks.

A more principled approach would be to add some new magic primitive to PEP 484 (and support in mypy of course) that would allow one to write a stub for attrs that explains to mypy what the effect of the class decorator is. This might be easier now than when this issue was created given that we already have some support for metaclasses now (#2475), so some of the necessary scaffolding in mypy already exists.

You could also imagine that mypy could just parse the full source code for the attrs package and understand what it does without needing any help. But I think that's unlikely to be successful, due to the difficulty for mypy to understand the dynamic tricks such an implementation undoubtedly uses. (I haven't looked.)

In general, supporting code that does runtime manipulation of interfaces is not high on the list of things that mypy and PEP 484 are likely to support soon. It's generally a bit antithetic to the idea of static types, after all. :-)

Yup, I was just trying to be more specific about the issue here.

I'm not sure I agree that attrs (or at least its functionality) is "antithetic to the idea of static types". I understand why it creates challenges here, but the degree to which it does dynamic things is limited to one time when the class is created, not some random manipulations as the program is running.

That said, I'm fairly new to mypy and the typing system.

I agree that being able to write a stub would be useful absent more robust but considerably harder to implement solutions.

I'm not sure I agree that attrs (or at least its functionality) is "antithetic to the idea of static types".

I agree with this assessment. attrs itself is not antithetical, though it's current implementation obviously is. It's really about supporting better use of types, by making them simpler to declare and be useful. I'm sure that if there were a way to support type systems at the same time, that would definitely fall in line with the goals of attrs.

Whether the real solution is pluggability for the type system to allow for this kind of compile-time transformations, or whether the only real solution is to have something that fulfills the goals of attrs built into the language, I'm not sure, but they are two approaches that are both trying to encourage the proper use of types. The dynamic implementation is just that, an implementation detail.

Well, mypy doesn't execute the program. It just reads the source code. So in order to validate the ThingWithPath() calls it would have to know the signature of the __init__() method that is added by the @attrs decorator, which in turn depends on going over the class attributes looking for expressions of a whatever type is returned by attrib(). Figuring that out presumably requires understanding the attrs package thoroughly, which is not possible given the state of the art and mypy's limited understanding of source code.

If it helps anyone, we solved this problem recently (specifically with attrs) where we define the init ourselves for attrs. Maybe it could help someone else too! Reusing the example from above:

@attrs(init=False)
class ThingWithPath(object):
    _path = attrib(validator=instance_of(Path))

    def __init__(self, _path):
        self._path = _path

    def exists(self):
        return self._path.exists()

There's discussion on an API in attrs (here: python-attrs/attrs#151) that would help bridging this. I'm following along since I like both typing and attrs for removed (and always correct!) boilerplate.

@fxfitz

@fxfitz: As @ambv notes, the problem there is that your solution removes one of the big benefits of attrs. And presumably that prevents attrs from doing the validation it would have done in the generated __init__, no?

I suppose that if attrs nuked and replaced your __init__, that may work, and then you'd instead write:

@attrs()
class ThingWithPath(object):
    _path = attrib(validator=instance_of(Path))

    def __init__(self, _path: Path):  # Note added type annotation here
        pass  # This implementation isn't used, so pass

    def exists(self):
        return self._path.exists()

It would be confusing for someone who may put code into __init__ not knowing it's ignored… so it's not 100% great, but maybe better.

@wsanchez

@wsanchez Ah right, I forget that if you init=False you would lose your validation; my bad! We used a similar approach to what I suggested earlier, but our goals were to (1) get mypy working and (2) still use attrs for the working equality/hash checks.

Hello, I'm one of the attrs maintainers.

I think it's fair to say mypy will have a really hard time ever working out of the box with attrs since we do some really filthy things under the hood.

I also think making attrs work with mypy would be a very worthwhile thing to try to do, since the point of attrs is to make people use structured data more and this is very complimentary to the idea of static typing checks.

I'd be interested in maybe being involved with the effort to develop a plugin API, if that's the direction we're going in, and ultimately developing a plugin for attrs.

Great plan! I wonder if you could approach this from the other direction: first write a mypy patch that supports attrs, hard-coding recognition of the special constructs it supports; and then, learning from that experience, figure out how a plugin API should be designed.

I'll give it a shot. No promises on the timeline :)

Oh wait. Just re-read the thread:

Concerning the plugins, I already mentioned this, I would prefer not to allow plugins to replace TypeInfos (because of problems with forward references), but only allow to modify TypeInfos in-place (for example update names symbol table, or similar).

ClassDecoratorContext = NamedTuple(
    'ClassDecoratorContext', [
        ('decorator', Expression),
        ('cls', TypeInfo),
        ('api', SemanticAnalysisPluginInterface)])

And the function will return None I guess.

@euresti

@euresti

You mentioned that we should only be modifying TypeInfo but none of the plugins take TypeInfos they all take Type or Expression.

Yes. You will need to add a new plugin kind. If you have time, I think it makes sense (to be future proof) to ensure that plugins can be triggered not only by a specific decorator full name, but also by a specific metaclass or base class full name.

Oh wait. Just re-read the thread:

Yes, this looks like a reasonable API for the plugin. The plugin should modify TypeInfo in place and return None. I think however it is better to pass the whole ClassDef AST node (again, to be future proof) then the decorator expressions can be easily extracted by the plugin when necessary.

RFC: Add Class Decorator/Metaclass/Base Class plugin

@ilevkivskyi

@ilevkivskyi Just added an RFC PR. (Sorry for pinging, I don't know if github notifies when PRs are added)

@Tinche

@Tinche @hynek Now that the plugin scaffolding is nearly finished I started working on the actual plugin. You can see the code here: https://github.com/euresti/mypy/blob/attrs_plugin/mypy/plugin.py#L421

It actually works!

import attr
import typing


@attr.s(auto_attribs=True)
class Auto:
    normal: int
    _private: int
    def_arg: int = 18
    _def_kwarg: int = attr.ib(validator=None, default=18)

    FOO: typing.ClassVar[int] = 18

reveal_type(Auto)
Auto(1, 2, 3)


@attr.s
class Typed:
    normal: int = attr.ib()
    _private: int = attr.ib()
    def_arg: int = attr.ib(18)
    _def_kwarg: int = attr.ib(validator=None, default=18)

    FOO: int = 18

reveal_type(Typed)   
Typed(1, 2, 3)


@attr.s
class UnTyped:
    normal = attr.ib()
    _private = attr.ib()
    def_arg = attr.ib(18)
    _def_kwarg = attr.ib(validator=None, default=18)

    FOO = 18

reveal_type(UnTyped)
UnTyped(1, 2, 3)

Which now gives the following output from mypy:

foo.py:14: error: Revealed type is 'def (normal: builtins.int, private: builtins.int, def_arg: builtins.int =, def_kwarg: builtins.int =) -> foo.Auto'
foo.py:27: error: Revealed type is 'def (normal: builtins.int, private: builtins.int, def_arg: builtins.int =, def_kwarg: builtins.int =) -> foo.Typed'
foo.py:33: error: Need type annotation for variable
foo.py:34: error: Need type annotation for variable
foo.py:35: error: Need type annotation for variable
foo.py:36: error: Need type annotation for variable
foo.py:40: error: Revealed type is 'def (normal: Any, private: Any, def_arg: Any =, def_kwarg: Any =) -> foo.UnTyped'

But I'm not sure if this code belongs in mypy or in attrs or maybe in a completely different project altogether? Also the pyi file has to go somewhere but I'm not sure if it goes in the typeshed or somewhere else. Please advise.

Also I imagine dataclasses will look pretty similar to the auto_attrib case.

Cool! I can take a deeper look when I have time.

Now might be a good time to consider where this code might actually live. Does mypy support this plugin living outside and somehow picking it up after it being pip installed? We will want to have this plugin versioned according to which attrs version it can support.

Great work David, I'm super excited that someone found the time to pull this thru!

I think part of the question where to put it also kind of depends on how the plugin APIs will evolve? It would be unfortunate if certain versions of attrs only worked with certain versions of mypy.

So I think it might make sense to to put it either into mypy or start a new package in the python-attrs orga? attrs proper seems like the worst possible place at this point.

@euresti

@euresti Stub files should go to typeshed. After PEP 561 is accepted and implemented, you can host the stub wherever you want and distribute them via PyPI/pip install.

As for the plugin it is less clear. Currently, mypy doesn't support plugins installed via pip install, but I think that the long term goal is to support this. Therefore, there are three options:

First add the attrs plugin for mypy as a PR to mypy, then implement the installable plugin infrastructure.
First implement the full infrastructure, then publish the attrs plugin on PyPI.
Work on two projects in parallel.

I think option 1 is the fastest way to get the attrs support. I am leaning towards this option, but I don't have a strong preference.

I think option 1 is the best for both projects until your plugin API stabilises. In the end you’ll get failing tests if you break something. :)

Once the plugin API is solid, we can talk about extracting it into a separate project.

attrs proper seems like a terrible option in every way.

I agree that including the plugin with mypy is the right course of action. It may be a while before mypy supports installing plugins through pip.

@euresti

@euresti OK, it looks like everyone likes the first option, so go ahead and make a PR to mypy.

Add Class Decorator/Metaclass/Base Class plugin (#4328)

Plugin to typecheck attrs-generated classes (#4397)

Navigation Menu

Repository navigation

Typechecking attrs-generated classes #2088

23 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Typechecking attrs-generated classes #2088

Description

Activity

gvanrossum commented on Sep 2, 2016

wsanchez commented on Mar 8, 2017

gvanrossum commented on Mar 8, 2017

wsanchez commented on Mar 8, 2017

ryanhiebert commented on Mar 8, 2017

gvanrossum commented on Mar 8, 2017

fxfitz commented on Mar 10, 2017

ambv commented on Mar 10, 2017

wsanchez commented on Mar 10, 2017

fxfitz commented on Mar 10, 2017

Tinche commented on Mar 23, 2017

gvanrossum commented on Mar 23, 2017

Tinche commented on Mar 27, 2017

23 remaining items

euresti commented on Dec 5, 2017

ilevkivskyi commented on Dec 5, 2017

euresti commented on Dec 6, 2017

euresti commented on Dec 14, 2017

Tinche commented on Dec 14, 2017

hynek commented on Dec 14, 2017

ilevkivskyi commented on Dec 14, 2017

hynek commented on Dec 14, 2017

JukkaL commented on Dec 14, 2017

ilevkivskyi commented on Dec 14, 2017

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions