[RFC] Quo vadis re: classes

[hynek]

Time for more existential questions.

Currently attrs has involuntarily two ways of attaching its methods. Both come with certain problems and there are (or are suspected) remedies.

We should really discuss this once and for all and transition towards one approach. Here’s what I’ve got:

We tack our methods to the existing class body.

Default behavior.

Problems

• Edge cases may appear because Python runtime is confused. As far as I can tell, so far only the inconsistence in the behavior of __eq__ and __hash__ (cf. ) is a real problem so far.

Remedies

• Catch all edge cases. So far it’s only one that popped up so it seems manageable?

We create a new class in @attr.s and return it instead of the original

Happens if slots=True. This has the upside that we create a brand new class that doesn’t change so no edge cases are to be taken care of.

However, the user gets a different class, which comes with its own set of problems.

Problems

• super() doesn’t work. TypeError when using super() and slots=True #102

Solutions

• ctypes voodoo

---

At this point, I find the confusingly broken super() worse than the default approach so I would prefer if we found a way to cheat __slots__ into existing classes and I’m not afraid of using C or meta programming to get there.

---

If others have more problems/solutions, I’ll happily add them.

[Tinche]

The problem with tacking methods onto an existing class is we can't add "slotness" to an existing class, right?

I'm more interested in the user-facing API than the internals; there's going to be filth under the hood either way. All other things being the same, I'd prefer to keep the dirt on our side, and let users frolic in their ignorance of what we're doing in their name.

I'm assuming adding slotness with a metaclass would require users to actually specify that metaclass? Like this:

@attr.s
class C(metaclass=SlotsMetaclass)
    a = attr.ib()

Personally I find the current approach of attr.s(slots=True) very nice since I can do things like

def profile(cl):
    return attr.s(frozen=True, slots=True)(cl)

@profile
class C:
    a = attr.ib()

and it's nice and elegant. Also does using a metaclass for slotness means users can't use another metaclass?

[hynek]

I absolutely agree on the filth being on our side.

My fear is just that the filth might just…break eventually? It seems like playing with internals that may also have other side-effects like confusing PyPy's JIT etc. :|

I really don’t know. 😖

[hynek]

So I tend to go with “cheating the interpreters and return new classes like slots=True” again.

Unless someone voices disagreement (@glyph?) we’ll ship the hacks in 17.3.0 and eventually move there in all cases.

[Tinche]

My fear is just that the filth might just…break eventually? It seems like playing with internals that may also have other side-effects like confusing PyPy's JIT etc. :|

I know this fear very well :) Mentally I counter it by saying to myself "we're a volunteer open source project, we're doing the best we can, if we do something wrong or paint ourselves into a corner with a wrong decision oh well, we'll fix it and try again" :)

👍 for returning new classes. I can put together a fix for #102 when I have some time and we can take it from there.

[hynek]

You've got any timeline in mind? Seems like your plate keeps getting fuller. :)

[hynek]

Since this is settled now, we have to figure out a road map on how to make it “always new class”.

That’s a breaking change re: at least the hashing thingie, so we’ll have to be careful.

[glyph]

What is the plan for being careful?

[hynek]

What I meant is that we won't do the bandaid thing again and try to find a way to do some kind of deprecation cycle.

Maybe make it an attr.s Option so we can raise a DeprecationWarning?

[hynek]

So we’ve just ran into more problems re metaclasses and I’m afraid taking the “always create new class” is just not practical. We’d break wayyy to much stuff on the way.

So the question is: what can be done instead?

• find some clever hack to get __slots__ on existing classes
• @RonnyPfannschmidt mentioned metaclasses but I know nothing about how they could help

[RonnyPfannschmidt]

sanely using meta-classes would require to inherit from a class that is a instance of them
since they are the then completely involved in the class construction, there is no need to modify a class, all preparation can be done before constructing the class and stuff like super() would just work as far as i understood (the class would never be taken away from under it)

[Tinche]

The original problem with metaclasses is that they aren't composable, right?

Our current problem is replacing the class won't allow our users to use the metaclass they want. But switching attrs to a metaclass approach again won't allow our users to use the metaclass they want :)

[RonnyPfannschmidt]

true - but when using attrs, many things you do with metaclasses are displaced anyway

for example if you use a sqlalchemy declarative metaclass, you dont want any attrs behaviour

so i dont see enabling metaclass based inheritance for stuff like correct __slots__ support and correct super() a bad thing

[Tinche]

Unfortunately, I think the approach of recreating the class is fundamentally incompatible with metaclasses. We might make it work in some cases but not in all cases.

So maybe we can brainstorm a better API for slots, and I can replace our black magic with different black magic so we set slots before the class is finalized. Then we would never have to replace the class.

Should we open a new issue to talk about different slots APIs?

[hynek]

I think this issue is fine since we don’t have anything else open. We can open something new once something concrete starts crystallizing.

[RonnyPfannschmidt]

well, metaclasses are fundamentally a mechanism to make the need to recreate the class to begin with go away - since the initial instantiation of the class instance would already do the work, where attrs currently has to break the world by moving the actual class away from under its methods

[Tinche]

What if we could make this work:

@attr.s
class C:
    attr.slots()    # Could take parameters, like attr.slots(weakref=True)
    a = attr.ib()

Option two:

@attr.s
class C:
    __slots__ = attr.slots()
    a = attr.ib()

But honestly the dunder slots just looks ugly to me, like an implementation detail leaking out.

[hynek]

Option one looks better but I could live with both I guess?

[oremanj]

I think you could support attr.slots() in the class body using the trace hook:

In [35]: def slottify(frame, old_trace):
    ...:     old_local_trace = [frame.f_trace]
    ...:     def tracer(frame, event, arg): 
    ...:         if event == 'call': 
    ...:             if old_trace is not None:
    ...:                 return old_trace(frame, event, arg)
    ...:             return None
    ...:         if old_local_trace[0] is not None:
    ...:             old_local_trace[0] = old_local_trace[0](frame, event, arg)
    ...:         if event == 'return': 
    ...:             frame.f_locals['__slots__'] = ('a',) 
    ...:             frame.f_locals['example'] = 'hi' 
    ...:             print(frame.f_locals) 
    ...:             sys.settrace(old_trace) 
    ...:         return tracer
    ...:     sys.settrace(tracer) 
    ...:     frame.f_trace = tracer 
    ...:                                                                                                                                                                        

In [36]: def slots(): 
    ...:     slottify(sys._getframe(1), sys.gettrace()) 
    ...:                                                                                                                                                                        

In [37]: class A: 
    ...:     foo: int = 1 
    ...:     bar: str 
    ...:     slots() 
    ...:     baz: float 
    ...:     quux: bytes = b"wat" 
    ...:                                                                                                                                                                        
{'__module__': '__main__', '__qualname__': 'A', '__annotations__': {'foo': <class 'int'>, 'bar': <class 'str'>, 'baz': <class 'float'>, 'quux': <class 'bytes'>}, 'foo': 1, 'quux': b'wat', '__slots__': ('a',), 'example': 'hi'}

In [38]: a = A()                                                                                                                                                                

In [39]: a.a = 'yo'                                                                                                                                                             

In [40]: a.b = 'hi'                                                                                                                                                             
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-40-28f4f01f5c2c> in <module>
----> 1 a.b = 'hi'

AttributeError: 'A' object has no attribute 'b'

In [41]: A.a                                                                                                                                                                    
Out[41]: <member 'a' of 'A' objects>

In [42]: A.example                                                                                                                                                              
Out[42]: 'hi'

Obviously the code that executes on 'return' here is just an example -- the actual version would need to collect the attribute names to put into __slots__, save the defaults/attr.ib parameters somewhere that @attr.s would know to look for them, detect if it's called outside a class body, etc. But I think it should be feasible to implement. I might take a stab at a PR if the approach seems reasonable?

[glyph]

I'd like to use attrs with e.g. Brython. Depending on internal implementation details like these (ctypes, stack walking) seems doomed to become very very interpreter-specific.

@oremanj in addition to being interpreter-specific, doing that sort of thing makes PyPy really slow. Maybe it'll only make it slow at import time, but importing my codebase at work already takes a good couple of seconds and I'd rather not stretch it out even more.

[oremanj]

I think maybe a useful question here is, when are limitations that exist only on some interpreters worse than limitations that exist on all interpreters? (This isn't intended to be a straw man -- there's a maintenance cost to having code that's exercised only on some interpreters -- but since attrs already has some interpreter-specific code, I'm inferring that that maintenance cost has at least sometimes been judged to be worth it.)

Brython doesn't really support __slots__ anyway; it seems to implement them as a restriction on which attribute names can be set, but the attributes still go in the instance __dict__. attrs could choose to do something similar as its fallback mechanism for slots=True on unusual interpreters, or it could fall back on its current approach of re-making the class.

Performance is definitely an open question. (At a rough glance on a couple small inputs, the use of the trace hook looked better on CPython than attrs' current approach.) My understanding of PyPy is that most of the performance impact of getframe/settrace/etc comes from disabling the JIT, but I wouldn't expect the JIT to help much with class creation anyway, so I'd be surprised if the effect is as dramatic as you're worried about? Of course there's no way to know for sure without fully implementing it and timing it.

[glyph]

I'm definitely OK with the answer for Brython being "implement a new backend". I wish I knew how to set up CI for something like attrs that could meaningfully verify it against some JS / frontend runtimes. (Brython just being an example here; maybe VOC is a better one.)

The main thing is that I want to avoid jumbling up a gross interpreter hack in the interface to slotsification, i.e. an un-adorned function call at class scope being able to identify the defining class, necessitating stack-walking and other hacks to make it work, as opposed to something abstract in the decorator.

[oremanj]

Thanks, that's useful feedback and makes sense when put that way. I suspect similar magic can be performed from the decorator, since the decorator gets evaluated before the class definition, but I haven't looked in detail yet.

[oremanj]

I brainstormed some more about where we could insert the magic to make this work. I think the most promising options involve taking advantage of the fact that if you pass metaclass=foo where foo is a callable but not a type, foo gets called directly without calculating a most-derived metaclass. We could provide a callable that fiddles with __slots__, then determines the "real" metaclass and calls it. This attrs-provided "metaclass" would only be used at class definition time and would not be visible to inspections of the generated class. I'm limiting my analysis to Python 3+; I'm not sure what attrs' timeframe is for dropping Python 2 support but I don't see any reason Python 2 couldn't continue to use the existing slots logic with its metaclass limitations.

The easiest and least magical approach would require the user to put something in their class's bases list explicitly. This only uses stuff in the official Python language definition so it should theoretically be portable to all interpreters that implement py3. Saying slots=True in the @attr.s argument list would continue to produce the current behavior of re-creating the class. Simple example: the above C class would be written something like

@attr.s
class C(metaclass=attr.slotted):
    a = attr.ib()

which could support an alias

@attr.s
class C(attr.slotted):
    a = attr.ib()

in order to be less daunting for users who don't want to think about the word "metaclass". (The purest version of this latter base class form would only work when none of C's other bases have nontrivial metaclasses, though support could be added for GenericMeta and ABCMeta without much trouble. It might even be possible to support all metaclasses on 3.7+ using __mro_entries__.)

More complex example: if you had the non-slotted attrs class

@attr.s
class C(A, B, metaclass=Meta, keyword=42):
    ...

and you wanted to make it slotted with no weakref slot, you would write something like

@attr.s
class C(A, B, metaclass=attr.slotted, attrs_weakref_slot=False, attrs_next_metaclass=Meta, keyword=42):
    ...

Other spelling options: metaclass=attr.slotted_with_metaclass(Meta), metaclass=attr.slotted[Meta].

More magic: Once the above exists, there's the option of additionally hooking builtins.__build_class__ (works on CPython and PyPy but isn't officially part of the language spec) to gain the ability to rewrite the bases list so the user doesn't need to explicitly write metaclass=attr.slotted or similar. This would add some slowdown to the creation of every class, not just attrs classes. There are a few different options for how the hook would decide whether to do the rewriting:

• Rewrite if the user passes attrs_slotted=True (or whatever name) in the base class list. That is, you'd write something like

  @attr.s
  class C(attrs_slotted=True):
      a = attr.ib()
  

  This seems no less ugly than inheriting from or using-as-a-metaclass attr.slotted, so I don't think it's a winning approach, but I mention it for completion.
• Rewrite if there is a attr.s(slots=True) object (with no these) that has been created in the current thread but not yet applied to a class. (Class decorators are evaluated before the class body and called afterward.) This would provide the benefits of the new approach (better metaclass treatment) without requiring code changes, but at the cost of confusion if anyone creates other classes between creating a attr.s object and creating the class it's to be applied to. For example, if you write:

  def evil_decorator():
      class A: ...
      def decorate(cls): ...
      return decorate
  
  @attr.s(slots=True)
  @evil_decorator()
  class C:
    ...
  

  then the slottedness applies to evil_decorator.<locals>.A rather than to C. It would be possible to detect this when applying attr.s to C, but at that point it's too late to fix the issue (all you can do is raise an exception).
• Rewrite if the next bytecode instruction after the call to attr.s is LOAD_BUILD_CLASS. This would have fewer negative externalities than the previous bullet, but would also miss more valid cases (such as calls to attr.s from a wrapper function). It also carries the downsides of stack introspection. I think the resulting "will it or won't it work with my metaclass?" uncertainty probably makes this a non-starter.

The trace hook could be used instead of hooking __build_class__, but I think it would carry most of the same downsides (not having a good way to tell which class body goes with which @attr.s decorator).

My gut feeling having written all this out is that the "more magic" approaches that could support unmodified @attr.s(..., slots=True) will be subject to a long tail of weird corner case problems. The earlier-discussed explicit metaclass approach represents an API change but is more portable and reliable. What do you think?

[hynek]

Oof there's a lot to unpack here. :)

So first of all, Brython is not gonna hold us back from having a better experience on the most used Python. We do have a portable solution right now and having a good experience on Py3+ should be worth some extra code.

That said, I think the first approach won't work because __slots__ need to be set before the class is created but I think you realized it in one of your comments too.

Ultimately I'm afraid we won't get around metaclasses unless CPython adds programmatic slots support (which they kinda thought about).

Having to subclass is obviously my least favorite approach; ideally everything would be done in the decorator. I wonder if something could be done with a custom __new__? I remember playing with it but don't remember what stopped me from actually using it. 🤔

P.S. we absolutely won't be rewriting bytecode but I appreciate the level of evilness. 😂

[hynek]

I think we'll get the necessary knobs from CPython sooner or later so closing this.