attrs icon indicating copy to clipboard operation
attrs copied to clipboard

Class variables

Open hynek opened this issue 7 years ago • 51 comments

Every now and then I stumble into wanting to have class variables that are part of everything (e.g. repr) except __init__.

We can do:

@attr.s
class C:
    x = attr.ib(default="42", init=False)

but that seems wasteful.

It would be nice to have something like:

@attr.s
class C:
    x = attr.ib_class(42)

that just promotes the value into x in class level and adds an Attribute to attrs's bookkeeping.


Opinions?

hynek avatar Jul 22 '17 09:07 hynek

Somehow this doesn't sit right with me for reasons difficult to articulate before my brain processes this a little more.

A class variable wouldn't be part of __init__, __hash__ or __eq__ right?

Tinche avatar Jul 22 '17 12:07 Tinche

Well that’s the big question how to actually treat them. You can overwrite class variables in Python too and then they become instance variables:

>>> class C:
...     x = 42
>>> i = C()
>>> i.x = 23
>>> C.x
42
>>> i.x
23

Maybe all I need is an option that pushes the value on the class and otherwise works as usual?

class C:
   x = attr.ib(default=42, class_var=True)

which would be equivalent to:

class C:
    x = 42

but otherwise it’d be a regular attrs attribute? I like this one better indeed. 🤔

hynek avatar Jul 23 '17 07:07 hynek

Ah, I see. This technique of using a class variable as a default fallback for an unset instance variable is interesting; I have to admit I only learned of it a few months ago. I use class variable in different ways so I wasn't sure what use case you had in mind, this clears it up.

This is a way of conserving memory, right? Instead of a million instance dicts having a key set, the key is only in the class dict, but it can be overridden on a particular instance if needed.

The first thing that comes to mind is slot classes can't really support this, and they are generally the more memory-efficient option. Of course if your class has a hundred default fields, doing it this way is going to be more efficient than a slot class with a hundred slots, so it's worth considering.

These attributes must have a default, the class must be non-slots, and they would be handled differently in two ways:

  • attr.s sets the actual class variable to the given default
  • in the generated __init__, if the value is not provided, do nothing.

All the other generated methods can continue using self.x as if there's no difference. What about factories? If the factory takes self, that's invalid too I think.

Since these attributes must have defaults, what about this for syntax:

@attr.s
class A:
    x = attr.ib(default=class_var(42))

(if not, your suggestion with default=42, class_var=True is good.

Tinche avatar Jul 23 '17 22:07 Tinche

It seems like overloading attr.ib, versus adding a new attr.ib_class may become increasingly problematic. It's already clear here that there are different behaviors in play here, so a lot of arguments and options that attr.ib accepts are not compatible with the class variable behavior.

For example, if attr.s sets x to 42 on the class, then the Attribute object is gone, right? So adding a validator would not work, but you might think that's a thing you want, given that a use case noted about is that you might want to set the value on an instance of the class… But my point is that validator= may not make sense there, and you'd end up with some switching logic in attr.ib that may unnecessarily complicate it.

wsanchez avatar Aug 02 '17 16:08 wsanchez

Just quickly:

  • Attributor access on the class body is deprecated and will be relived this year anyway.
  • Class attributes can be overwrites within instances and not affect the class which makes them kind of useful and doesn't preclude validators.

However they're still pretty special so I tend to an own attr.ib_class still.

hynek avatar Aug 02 '17 17:08 hynek

For example, if attr.s sets x to 42 on the class, then the Attribute object is gone, right?

This is already deprecated behavior. C.x will either be a slot member descriptor (a Python thing) if the class is a slot class, or an AttributeError if the class is a dict-class. I think we're already out of the depreciation period, so the next release should be OK for messing with C.x? (The canonical way of getting to the Attribute is attr.fields(C).x.)

So adding a validator would not work [...]

It'll work.

This is the equivalent of what is being proposed:

class C:
    x = 42

    def __init__(self, x=NOTHING):
        if x is not NOTHING:
            self.x = x
            __attr_validator(self.x)

Then you get:

>>> C()
C(x=42)

with C().__dict__ being empty, which is the point.

Tinche avatar Aug 02 '17 17:08 Tinche

OK, y'all in deeper in the black magic that I've normally gone, so my assumptions are off. Uh… thanks for doing that do I don't have to. :-)

wsanchez avatar Aug 02 '17 22:08 wsanchez

There's a bit you left out in the code you say this is equivalent to, which is what happens when you say:

>>> c = C()
>>> c.x = "some value"

I'm assuming "magic, duh", but noting it for completeness.

wsanchez avatar Aug 02 '17 22:08 wsanchez

Oh, I guess for more complete completeness:

C.x = "another value"

Which I assume is out of the attrs problem scope…?

wsanchez avatar Aug 02 '17 23:08 wsanchez

Well Hynek kinda explained it in https://github.com/python-attrs/attrs/issues/220#issuecomment-317234547. This is just normal Python, feel free to copy/paste the code, play around with it and discover exactly how it works ;)

Tinche avatar Aug 02 '17 23:08 Tinche

@Tinche deprecation period ends 2017-08-30…can’t wait!

Originally, I wanted to get 17.3 out asap but I think I'll wait for this to pass. Guess I’ll be releasing out of South Africa… :)

hynek avatar Aug 08 '17 05:08 hynek

This is already deprecated behavior. C.x will either be a slot member descriptor (a Python thing) if the class is a slot class, or an AttributeError if the class is a dict-class.

If C.x will no longer hold an Attribute I would expect the next best thing for it to hold would be the default value. This would also play nicely with static type checking, where both C.x and c.x are expected to be the same type. Also, it's a common idiom in static type checking to define attribute defaults and their types at the class level.

Where can I catch up on the discussion surrounding this proposal?

chadrik avatar Aug 29 '17 16:08 chadrik

Which discussion exactly? The removal of Attribute will happen before the next release FWIW.

And yep, we probably could just pass immutable default values thru into class variables? I wonder if that’d have any weird side-effects, because otherwise it'd be a nice speed boost.

(P.S. as a general disclaimer: I'll be off the grid Sep 1–17 plus on vacation until Sep 24. Right now I’m having hell at work due to preparations for that so I’m less responsive than I’d like to be and it’ll be worse in the near future. OTOH I hope I’ll get a bunch of FOSS work done afterwards because I’ll be a kind of retreat until Oct 18)

hynek avatar Aug 29 '17 16:08 hynek

Which discussion exactly? The removal of Attribute will happen before the next release FWIW.

I was just wondering if there was a publicly-viewable discussion (or code) around the removal of Attribute access from attrs classes, in favor of the new (descriptor-based?) design. It sounds like the plan has been fairly well fleshed out, so I thought I'd do you the favor of getting caught up before jumping in with suggestions that you've already thought of.

chadrik avatar Aug 29 '17 17:08 chadrik

It happened pretty much exactly a year ago by that decision is orthogonal to this ticket. It was just a bad idea to attach more than necessary to classes. There's also no real plans around descriptors. Current plan is to just delete the attr.ibs and leave the class clean (or for slots classes: not attaching anything in the first place.)

hynek avatar Aug 29 '17 17:08 hynek

So can someone explain why we should include class variables in the attrs system?

agronholm avatar Nov 04 '17 10:11 agronholm

Quoting Hynek from the other thread:

In non-slots you can overwrite class variables and they become instance variables, see #220 for a discussion. It kind of gives you “free” defaults.

Yes, but this still doesn't explain why the class vars should be attr()ibuted. They work quite well without it.

agronholm avatar Nov 04 '17 10:11 agronholm

You’re definitely on to something. We could argue whether or not attrs should support it at all, however I think you’re making a good case to make it opt-in.

IOW, the API would go more like:

@attr.s(auto_attribs)
class C:
    x: ClassVar[int] = attr.ib(default=42)

because collecting it would be impossible to prevent undesired class vars to be collected without setting it to attr.ib() which is a bad API.


The important point here is that that would make #220 not a blocker for 17.3 anymore.

hynek avatar Nov 04 '17 10:11 hynek

Yes, but this still doesn't explain why the class vars should be attr()ibuted. They work quite well without it.

If it’s just a free default, you def want it as part of your reprs and cmps because they can vary wildly.

hynek avatar Nov 04 '17 10:11 hynek

I don't think ClassVar and "free" defaults should be mixed. If you want this, why not just have attrs set the default value directly in the class?

agronholm avatar Nov 04 '17 10:11 agronholm

Or at least give the user the option to do so.

agronholm avatar Nov 04 '17 10:11 agronholm

To my understanding (and in my opinion), something annotated with ClassVar is never intended to be set as an instance variable. Otherwise you could just do foo: int = 4.

agronholm avatar Nov 04 '17 10:11 agronholm

That was kind of the original plan of this ticket. :)

But yeah, I think we’re in agreement, that this is not a blocker for 17.3 in any way. phew

hynek avatar Nov 04 '17 10:11 hynek

Are we in agreement about how to handle ClassVars with auto_attribs=True then?

agronholm avatar Nov 04 '17 10:11 agronholm

I think so. :)

hynek avatar Nov 04 '17 10:11 hynek

We now have ClassVar type in typing: https://docs.python.org/3/library/typing.html#typing.ClassVar

It would be awesome to have this clean API:

@attr.s(auto_attribs=True)
class MyClass(object):
    class_var: ClassVar[int]

    instance_var: str

Will result in:

  1. MyClass.class_var as a typed class variable
  2. MyClass().instance_var as a typed instance variable

sobolevn avatar Aug 04 '19 09:08 sobolevn

Isn't that exactly what happens? You just have to assign it a value:

In [7]: @attr.s(auto_attribs=True)
   ...: class MyClass(object):
   ...:     class_var: ClassVar[int] = 5
   ...:
   ...:     instance_var: str
   ...:

In [8]: MyClass.class_var
Out[8]: 5

In [9]: MyClass("hi").class_var
Out[9]: 5

Fields decorated as ClassVar are ignored.

hynek avatar Aug 06 '19 18:08 hynek

@hynek Related to this, would it make sense for the following to produce an error?

@attr.s(auto_attribs=True)
class MyClass(object):
    class_var = 5
    instance_var: str = 'foo'

Currently (in attrs 19.1.0) class_var becomes a class variable and instance_var becomes an instance variable, which seems a little unintuitive and a potential source of bugs, since accidentally forgetting a type annotation will give you a class variable instead of an instance variable. I just started using attrs and was confused by this for a while.

I think it would make sense for auto_attribs=True to require the explicit ClassVar[int] annotation, just like it requires attr.ib fields to be type-annotated.

kryft avatar Sep 24 '19 15:09 kryft

There's two things that's speak against that:

  1. Totally wrecks backward compatibility :)
  2. I find this against attrs's philosophy of staying out of your business unless told otherwise.

hynek avatar Sep 25 '19 08:09 hynek

would a warning when a class variable is not annotated as such be in the game of ensuring people correctly annotate intent

RonnyPfannschmidt avatar Sep 25 '19 09:09 RonnyPfannschmidt