Skip to content
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

why not just attrs? #19

Closed
glyph opened this issue Jun 24, 2017 · 21 comments
Closed

why not just attrs? #19

glyph opened this issue Jun 24, 2017 · 21 comments

Comments

@glyph
Copy link

glyph commented Jun 24, 2017

I don't mean this as a dig at the project here, but a serious question:

Is there any reason this project should exist?

Every aspect of the design here appears to be converging further in the direction of exact duplication of functionality available within attrs, except those which are features already on attrs's roadmap. The attrs maintainers have carefully considered a number of subtle issues related to this problem domain, and every discussion I have observed thus far on the dataclasses repo has paralleled the reasoning process that went into attrs's original design or its maintenance. (Remembering, also, that attrs is itself a second-generation project and there was a lot of learning that came from characteristic.)

I haven't had time to read all of the python-ideas thread, but all of the objections to attrs that I could find seem to have to do with whimsical naming or a desire to shoehorn convenient class creation into a "special" role that is just for "data" and not for regular classes somehow. I shouldn't be passive-aggressive here, so I should just say: I can't follow Nick's reasoning on #1 at all :-).

The silly names could be modified by a trivially tiny fork, if that is really a deal-breaker for stdlib adoption; honestly I find that the names grow on you. (More than one attrs user has separately come up with the idea that it is a lot like Python's significant whitespace.)

That said, of course there may be some reasons or some broader goal that I'm missing, but if this is the case it seems like writing a very clear "goals / non-goals / rejected approaches" section for the PEP itself would be worthwhile. The reasons given in the PEP don't really make sense; the need to support python 2 hasn't been a drag on python 3 that I'm aware of, and annotation-based attribute definition is coming to attrs itself; it's a relatively small extension.

@warsaw
Copy link

warsaw commented Jun 24, 2017

I have to say that I kind of like the cute names in attrs. Where I've used it it reads very naturally and doesn't cause you to scratch your head at the meaning.

But if the suggestion is to just bring attrs into the stdlib, while I'd support it, does Hynek?

@habnabit
Copy link

pip seems to be the correct model here: installed by default, but upgradeable. Please don't kill attrs by moving it into the stdlib.

@glyph
Copy link
Author

glyph commented Jun 24, 2017

if the suggestion is to just bring attrs into the stdlib, while I'd support it, does Hynek?

I can't speak for Hynek, so hopefully he'll pipe up himself. If he were unwilling for some specific process-related reason, that would certainly be something that could be written down in the PEP, though :).

@ericvsmith
Copy link
Owner

As I've said, I like the "cute" names. And clearly I like most of the design decisions in attrs: I'm switching my own code over to using it.

The current PEP, such as it is, is mostly "notes to self" that obviously needs a huge amount of work. The real world has prevented me from making much progress in the last few weeks.

I think it comes down to: if attrs only had to support >= 3.6, and didn't have any backward compatibility constraints, what would it look like?

Whether what results from that exercise are different enough from attrs and are worth putting in the stdlib are questions yet to be answered (or asked!).

@hynek
Copy link

hynek commented Jun 25, 2017

Alright, so:

  • It’s completely out of question to put attrs into the stdlib ipaddress-style and basically freeze it in time.
  • Going the simplejson/json route and adding it under a different name would make me rather sad, but at least people would be able to keep using attrs proper although the situation is confusing to many.
  • Having a versioned and upgradeable instance of attrs part of the stdlib – leaving open how that’s achieved – is something I could agree to.

The last point opens a question though: one reason why core is interested in this is that they’d like to use it themselves within the standard library. I’m sure it’s possible to achieve but there’s always the risk that an attrs update breaks the stdlib.


To address some of the concerns:

  • whimsical names: if it’s too much for the stdlib, I’ve already considered adding an attrs package from which you can import both attrs/attrib (serious business) as well as attr (attr.s/attr.ib). This would be another impulse to follow through. At this point I’m neutral on cute names but I’m annoyed about importing from attr . The history is that attrs vastly outgrew it’s original scope; for better or for worse.
  • attrs’s Python 2 support is an non-issue. attrs was always a Python 3-first project and both Tin and I have mostly Python 3 in prod. If we have to compromise, we do it on the expense of Python 2. The proposed usage of class annotations already works in a proof of concept – I just want to take it as an opportunity to iron out some little warts we’ve accumulated (mostly how slots – create new class with added methods – vs non-slots classes – tack on methods – are created).
  • serialization: part of integrating class annotations will also be to make types a first class citizen of attributes. @glyph has a working serialization lib in prod (not public yet as I understand), Tin’s cattrs is a thing too. Things like asdict mentioned in the serialization ticket seem beside the point to me: what we need is unequivocal type data we can work on. It’s well possible that this becomes an Python 3-only feature in attrs proper. #ohwell

Did I forget something?

@gvanrossum
Copy link

I would say the stdlib is lacking some very useful functionality in this area. Since it's it's not specific to to an application domain, it makes sense for the stdlib to provide it natively. PEP 526 gives a certain impetus to this desire, since now it might be possible to design something where you can simply write

x: int

(in some decorated class) as opposed to

x = attr.ib(validator=attr.validators.instance_of(int))

It would also probably be a good idea to have something in the stdlib so that people aren't tempted to abuse collections.namedtuple (or its typed cousing typing.NamedTuple) when they are looking for a notational shorthand.

It is possible that attrs is the "gold standard" for this functionality -- but it is neither the first nor the last library of this sort. And it should never be simply promoted to stdlib status -- that would be terrible for all involved. So I think it's entirely reasonable that we try to extract the good ideas from attrs, combine them with some other good ideas, and try to package them into something that befits the standard library, without trying to preserve any kind of backwards compatibility with attrs itself. All this should be done while exercising restraint in feature abundance, favoring careful design for extensibility over including some of the more advanced features.

The exercise is still in its early stages, but I see no reason to stop because attrs already exists.

@ericvsmith
Copy link
Owner

When I said "only supporting >= 3.6", I was specifically referring to PEP 526, variable annotations. I'll make this more clear when I get around to writing the PEP.

I'm hoping that by supporting static typing, we can avoid adding validators.

@dstufft
Copy link

dstufft commented Jun 25, 2017

Presumably attrs can support PEP 526 too though? I view it as a benefit that attrs can support both things like PEP 526 for nicer, cleaner code OR a more verbose mechanism for older Pythons. Largely because very few of my projects that I have ever worked on can mandate >=$LATEST_VERSION_OF_PYTHON but having both means developers can use whichever best suits their purpose.

Reinventing seems to make sense when the stdlib has some unique ability that an external project doesn't have (for instance, the ability to define new language features like async def or the actual language level support for isolation in venv or the ability to define some sort of standard like logging). However when something is being done entirely in "user land", I think an obvious question is are we doing something meaningfully better, or are we just doing something different?

It feels a lot like this is just doing something different to me, which the current (proposed) feature set seems to agree with. It appears to largely be a direct copy of most/all of attrs features, just with slightly different spellings and semantics (i.e. painting the bikeshed a different color). The PEP so far tries to answer why not just using attrs, but that currently only has two points and both seem like they are anemic post-hoc rationalizations rather than real reasons.

In fact, the two reasons seem to really be the same reason, which is the implication that by supporting older versions of Python, attrs is somehow worse off than if it could depend on newer language features. While it is true that attrs cannot depend on these language features as part of it's own implementation, it does not appear to be the case that consumers of attrs could not use these features for a cleaner, more concise API (and @hynek has indicated that is true).

Does that mean that seeing if we can do something better than attrs is a bad thing to do? Well no. Experimentation is great and can come up with new ideas, and possibly something much better than has already existed. However, before we actually commit to something, I think the default should be to look at how we can incorporate the thing that already exists unless we have some really compelling reason why that thing is unsuitable.

@gvanrossum
Copy link

No. Incorporating attrs is a bad idea.

@dstufft
Copy link

dstufft commented Jun 25, 2017

Why?

@gvanrossum
Copy link

Read the rest of the thread.

@dstufft
Copy link

dstufft commented Jun 26, 2017

I did read the thread, it contains no explanation of why it is a bad idea other than a claim that it is?

The closest thing to an explanation is a hypothetical of "well maybe if attrs only had to support Python 3.6 or newer, it would be even better!", but so far I don't think that can be born out (and even if it were so, it'd only mean a different trade off, not a bad idea).

@gvanrossum
Copy link

Wow. Lots of fans of emojis here. :-)

Hynek's three bullets above make a rather strong case against it. The third bullet ("a versioned and upgradeable instance of attrs part of the stdlib") leaves the way open but doesn't answer how -- the pip way is certainly not reasonable (what pip does is bundle a 3rd party package which can then be upgraded independent from the stdlib, which is fine for pip but not for something that the rest of the stdlib may depend on it). We've also had the xml stdlib module which explicitly supported installing extensions as a 3rd party package, and that turned out terrible. (People wrote code depending on the 3rd party package but didn't know it.)

But maybe I should also explain why I am against just incorporating attrs in the stdlib. I don't like the color it paints the bikeshed -- not the package name itself, not the "cute" names, not the basic way of declaring that something is an attribute (it should be an annotation, not an initialization). I like many of attrs' other ideas though, so I'd like to build a bikeshed for the stdlib in my own colors. If eventually Hynek likes the color scheme used by the stdlib he's welcome to repaint his shed.

Having a standard API will also make it simpler to add static type checking (PEP 484 style) for classes using dataclasses (or whatever name we pick, as long as it's not attrs :-). E.g. mypy should learn to infer the signature of the constructor added by the class decorator.

In general new stdlib modules should be designed with stdlib inclusion first and foremost in mind (like we did with asyncio). Almost every time a successful existing 3rd party module is proposed as a stdlib candidate we find it's not going to work.

@hynek
Copy link

hynek commented Jun 26, 2017

OK so I feel like I should set a few things straight.

First of all, I’m 0 on attrs inclusion in any form since it would probably mean more headaches for me – but I completely agree that to encourage proper OO design, writing proper classes should be more straightforward in Python. That’s why I’m here trying to help. To help, not push my ego or agenda or whatever – I really don’t feel like I have a dog in this fight.


One thing I’d like to stress is that many more people gave their input into attrs’s design evolution than on this PEP. So if you want to re-build my shed with colors that are more pleasing to your taste: I have no problem with that and will not fight you at all. The first rule of attrs is that it pops out regular classes. Thus there’s no lock-in and there’s no downside for me if you go your own way.

However I would strongly encourage you to stick to attrs’s design and priciples instead of reinventing the wheel from bottom up unless you come up with good reasons to diverge of course. Most of its design & features did not emerge from a vacuum but evolved from long discussions and there’s no need to replicate those here with fewer people, no matter how much smarter they are. As you may have noticed, your discussion tend to converge to the same results anyway. Also feel free to ask about our design decisions, chances are there’s a good reason for it. We’ve put a lot of effort into not painting ourselves into a corner, so be careful to not paint there yourself just for the sake of doing things differently.


Another thing that somehow keeps repeated and annoys me is PEP 526. As I wrote before, this already works in a PoC:

>>> import attr

>>> @attr.s
... class C:
...     x: int
...     y: attr.ib(default=42)

>>> C(1)
C(x=1, y=42)

>>> C("1", 23)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<attrs generated init 057634913447d4e148358419a9dd355e479f9105>", line 5, in __init__
    __attr_validator_x(self, __attr_x, self.x)
  File "/Users/hynek/Projects/attrs/src/attr/validators.py", line 33, in __call__
    attr, self.type, value,
TypeError: ("'x' must be <class 'int'> (got '1' that is a <class 'str'>).", Attribute(name='x', default=NOTHING, validator=<instance_of validator for type <class 'int'>>, repr=True, cmp=True, hash=None, init=True, convert=None, metadata=mappingproxy({})), <class 'int'>, '1')
("'x' must be <class 'int'> (got '1' that is a <class 'str'>).", Attribute(name='x', default=NOTHING, validator=<instance_of validator for type <class 'int'>>, repr=True, cmp=True, hash=None, init=True, convert=None, metadata=mappingproxy({})), <class 'int'>, '1')

It was trivial to add and will be in the next attrs release.

What’s also coming is first class support for types so people using legacy Python can use a construct like x = attr.ib(type=int) instead and we can work on nicer serialization solutions.

Please stop bringing up PEP 526.

@gvanrossum
Copy link

Agreed with most of what you said (esp. on keeping with attrs' design and principles -- though sometimes it's hard to know what's just bikeshed color and what's a design principle).

Regarding PEP 526, since you bring it up, I have to respond. I agree that there's no reason why attrs can't support PEP 526; but at least do it right. This is wrong:

y: attr.ib(default=42)

This misuses the annotation slot (the thing after : and before = if there is one). That slot should be used only for a PEP 484 type (or the ClassVar[] notation introduced in PEP 526). IMO the form to support is

y: int = attr.ib(default=42)

@vedgar
Copy link

vedgar commented Aug 13, 2017

Why not just

y: int = 42

?

@habnabit
Copy link

habnabit commented Aug 13, 2017

@vedgar
Copy link

vedgar commented Aug 13, 2017

(This is an answer to the previous version of the above post, that had http://www.attrs.org/en/stable/why.html#hand-written-classes as a link.)

You're quoting boilerplate code as an argument for writing

y: int = attr.ib(default=42)

instead of

y: int = 42

? I'm sure I misunderstood you, because that doesn't make any sense to me.

@vedgar
Copy link

vedgar commented Aug 14, 2017

Ok, so you edited your answer and I didn't receive notification. Something's weird on GitHub, but it's not your fault.

Anyway, I'm not quite sure what's your argument. If it is that ints might become mutable in some future version of Python, I don't find that credible.

If it is that you have to know whether your default is mutable to use such an implementation, that's true, but that's also true of your attr.s example (you use object itself in one case, but factory in another). So again you don't gain anything.

@ericvsmith
Copy link
Owner

I'm going to close this issue. I agree we should continue exploring "dataclasses" as a possible inclusion in the stdlib. I also think that attrs made some good design decisions, and that "dataclasses" will to some extent look like a stripped down attrs.

@ghost
Copy link

ghost commented Dec 25, 2021

attrs as Python, is like just lombok for Java, I like the lombok dedicated develop with Java, Also support the Attrs to dedicated develop with python.

But i surppose attrs shall be one python foundation supported project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants