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

Possibility to call custom initialization #13

Closed
ilevkivskyi opened this issue Jun 5, 2017 · 9 comments
Closed

Possibility to call custom initialization #13

ilevkivskyi opened this issue Jun 5, 2017 · 9 comments

Comments

@ilevkivskyi
Copy link
Contributor

Imagine a situation with this class:

class BigClass:
    def __init__(self, x, y, z, t, w, parent):
        self.x = x
        self.y = y
        self.z = z
        self.t = t
        self.w = w
        self.tmp = TempStorage()
        self.tmp.link(parent)
    ...
    def __repr__(self):
    # other boilerplate

It should be possible to refactor this into:

@auto
class BigClass:
    x: int
    y: int
    z: int
    t: int
    w: int
    def initialize(self, parent):  # We can choose other name, maybe name of the class?
        self.tmp = TempStorage()
        self.tmp.link(parent)

A simple way to achieve this is to add code like this at the end of generated __init__:

def __init__(self, <inserted args>, *extra_args):
    ...
    if hassattr(self, 'initialize'):
        self.initialize(*extra_args)
@hynek
Copy link

hynek commented Jun 5, 2017

JFTR attrs has a similar feature called __attrs_post_init__(self) and I would suggest to use a dunder method too, otherwise it looks like it’s part of a public API.

There’s really no reason to special-case extra_args though. Just let them put on self get them from there. It’s more straight-forward, easily understandable, and I doubt the complexity around handling extra_args is worth it. Especially because it would come with some asterisks.

@ilevkivskyi
Copy link
Contributor Author

I would suggest to use a dunder method too, otherwise it looks like it’s part of a public API

But it actually can be part of public API. Also dunder methods might look scary to users. Possible problem with using non-dunder method name is an occasional name clash. Possible solutions are:

  • Using a keyword (name to be discussed):

    from classtools import auto_fields
    
    @auto_fields(hash=True, setup='initialize')
    class BigClass:
        x: int
        ...
        def initialize(self, parent):
            self.tmp = TempStorage()
            self.tmp.link(parent)
  • A special decorator (name to be discussed):

    from classtools import auto_fields, on_create
    
    @auto_fields(hash=True)
    class BigClass:
        x: int
        ...
        @on_create
        def initialize(self, parent):
            self.tmp = TempStorage()
            self.tmp.link(parent)

    With this syntax it is possible to call multiple initializers (typically in the case where they are parts of public API that needs to be called on creation).

In both cases a user can start the name with a single underscore if this is not a part of public API.

There’s really no reason to special-case extra_args though. Just let them put on self get them from there. It’s more straight-forward, easily understandable, and I doubt the complexity around handling extra_args is worth it.

I am not sure what do you mean by "just let them put on self"? The only way to put something on self is to add a field for this. I could imagine situations where this may be unwanted. I have seen situations where __init__ has some flags that don't end up on self. Without extra_args there will be no way of adding some flags to generated __init__.

Also I don't think it will be difficult to add extra_args, especially that manual code generation is used. @ericvsmith What do you think?

@hynek
Copy link

hynek commented Jun 6, 2017

For reference, we had an epic bikeshed on that topic and decorators came up too: python-attrs/attrs#68 – I seem to remember that a well-known method had the least downsides – especially re subclassing.


Re extra_args

I’m using attrs syntax for simplicity here now, but you could do this:

@attr.s
class C:
    x  = attr.ib()
    parent = attr.ib()
    tmp = attr.ib(init=False)

    def __attrs_post_init__(self):
        self.tmp = TempStorage()
        self.tmp.link(self.parent)

The advantage is that it doesn’t introduce any new concepts. If you don’t want that field to be there, make it private using an underscore. OTOH I learned the hard way that trying to pander to every single use case under the sun leads to regret and tears. :) You can always write your own __init__.

Also this seems like a feature that can be added later if really necessary.

@ilevkivskyi
Copy link
Contributor Author

Indeed a special name looks most simple, but I don't like a dunder for this purpose. Beginners tend to think about dunders as being "magic" (although they are not) and therefore hesitate to use them.

@gvanrossum
Copy link

gvanrossum commented Jun 6, 2017 via email

@ericvsmith
Copy link
Owner

@hynek: that really was a mega-thread! Thanks for the pointer.

From reading the thread, I think an optional well-known entry point like __attrs_post_init__ does make the most sense. And do I understand correctly that it's left to the class authors to call super().__attrs_post_init__ if they want to? That seems like a good decision. As the arguments to the post init function never change, you don't have the same problem as trying to call __init__ through super().

So, I propose that (subject to name changes) we have the generated __init__ call __dataclass_post_init__, if one exists.

As far as extra parameters go: I'm not sure we can add it later, without changing the __dataclass_post_init__ function signature, which again runs in to problems with super() calls.

@gvanrossum
Copy link

I'll leave this for Eric to decide.

@hynek
Copy link

hynek commented Jun 7, 2017

And do I understand correctly that it's left to the class authors to call super().__attrs_post_init__ if they want to?

Yes, because everything else would end up in wonky guesswork. People can also call super().__init__() – that was one of the main original reasons why we added it in the first place (I think using PyQt was one of the use cases).

@ericvsmith
Copy link
Owner

I've implemented a parameter-less __dataclass_post_init__ function, that is called if present.

I opened issue #17 to decide if we want/need to pass parameters to it.

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

4 participants