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

Calling super.__init__ before attrs __init__ (or, __attrs_pre_init__) #725

Closed
indigoviolet opened this issue Nov 30, 2020 · 8 comments
Closed

Comments

@indigoviolet
Copy link
Contributor

indigoviolet commented Nov 30, 2020

Hi,

(Thanks for your work on attrs, it's fantastic, I'm a happy sponsor)

I'm subclassing pytorch's nn.Module and it has some "magic" treatment of members that are of certain types, which boils down to it requiring invoking super().__init__() before assigning instance variables, so I can't just use __attrs_post_init__ for this.

I've read #695 (comment), #640 (comment), #167 etc and I'm generally in agreement about composition vs. inheritance, but I'd love to be able to use the convenience of attrs in this situation: a library class that I need to subclass but have no control over.

Naively, it seems to me that this could be satisfied by having an __attrs_pre_init__ hook that receives all the constructor arguments. I assume you have considered this - would you be open to a PR adding it (it seems relatively straightforward following the code for __attrs_post_init__)?

@hynek
Copy link
Member

hynek commented Dec 1, 2020

Thank you for being a sponsor, happy to boot!

As stated in #393 (comment), I think it would be overall a nicer design to allow for telling attrs to write its __init__ into a __attrs_init__ that the user calls themselves?

That said I'm somewhat OK with doing __attrs_pre_init__ too, I'm just busy catching up with my issues on structlog right now – unfortunately I don't seem to have the mental capacity to do large-scale work on two big FOSS projects at once. 🙃

@Drino
Copy link
Contributor

Drino commented Dec 2, 2020

I would like to notice that while __attrs_init__ is a great feature, it forces user either to lose __init__ hints from IDE or to write boilerplate __init__ signature.

@hynek
Copy link
Member

hynek commented Dec 2, 2020

IOW types alone make it worth having both in any case?

@indigoviolet
Copy link
Contributor Author

Yes, my intention was to move current __init__ to be __attrs_init__, then autogenerate an __init__ like:

def __init__(self, ...args as before...):
     self.__attrs_init__(...args again...)
     self.__attrs_post_init__()

@indigoviolet indigoviolet mentioned this issue Dec 13, 2020
10 tasks
@hynek
Copy link
Member

hynek commented Dec 13, 2020

Ah I've missed this message! While I want it to do that conceptually, I don't want to have that extra method and and method call on all classes. attrs tries to be as lean and as fast as handwritten classes, so we'll have to live with some ugliness here so our users save bytes and µs. :)

@dojoteef
Copy link

@indigoviolet FYI that the approach I highlighted here works great as a DIY __attrs_pre_init__. I use it for Pytorch modules. In fact I've combined attrs with jsonnet to create configurations similar to AllenNLP. This allows for defining a model in a config file and instantiating it using attrs. Though, I went even farther and automatically register existing modules like nn.Linear. I even made a mypy plugin for it.

I'm looking to open source my code as a Pytorch configuration library built on attrs in the near future. But in the meantime, you should try the code I linked above.

@indigoviolet
Copy link
Contributor Author

@hynek : See #750 for __attrs_pre_init__ as you requested in #731 (comment) .

Just so I understand your thought process, earlier I thought you preferred the __attrs_init__ model over __attrs_pre_init__. Can you lay out the pros and cons of these approaches?

  1. I understand that a user-defined __init__ will not have the automatic type signature that the generated __init__ would have, so that's a downside of using __attrs_init__. In that sense __attrs_pre_init__ is nice.

  2. But when would I use __attrs_init__ instead of __attrs_pre_init__?

@hynek
Copy link
Member

hynek commented Jan 24, 2021

I think __attrs_init__ is nicer when you want do do more in your __init__ than setting values. E.g. deriving values for the initializer from different set of arguments and don’t want to span up a web of per-attrib converters and validators. I don’t quite remember because this topic has been on the agenda for many years (so many thanks for tackling it!!!) but I’ve seen valid arguments for both.

But in practice my gut feeling is that pre_init is gonna see more practical applications. But I might be wrong. There’s a sizable user base of attrs that chose us specifically because we don’t force types on anyone and once they’re out of the equation the upsides of pre_init wane.

Am I somewhat coherent?.

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