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

Suggestion: Consider changing the order for variable initialization in __init__ #1296

Open
germaniuss opened this issue Jun 14, 2024 · 1 comment
Labels

Comments

@germaniuss
Copy link

germaniuss commented Jun 14, 2024

First of all thanks a lot for the awesome library! I just recently discovered it and it saved me so many headaches. Recently I was trying to do something of this sort, where either x or y has to be given to the __init__ method for this to be valid:

@define
class MyClass:
    x: int = field()
    y: int = field()
    
    @x.default
    def y_plus_one(self):
        return self.y + 1
        
    @y.default
    def x_plus_one(self):
        return self.x + 1

This however (as the docs state) fails because self.y would not be initialized during the call to y_plus_one. Even though the docs state this fails I wanted to try and fix the issue and managed to do so in a very hacky way. However in the process I thought up a non invasive way in which this may work.

If we first initialize those fields that were given a value through __init__ and then use the NOTHING sentinel to initialize the values for those attributes that were not initialized through __init__ but before calling the factories, the problem dissapears. The reordered __init__ method would look something like this (I'm not really sure how the method is autogenerated but its a rough idea):

@define
class MyClass:
    ...
    
    def __init__(self, x = NOTHING, y = NOTHING): # <- this is autogenerated
        # initialize all fields before calling factories
        if x is not NOTHING:
            self.x = x
        elif isinstance(self.__attrs_attrs__['x'].default, Factory):
            self.x = NOTHING
        elif self.__attrs_attrs__['x'].default is not None:
            self.x = self.__attrs_attrs__['x'].default

        if y is not NOTHING:
            self.y = y
        elif isinstance(self.__attrs_attrs__['y'].default, Factory):
            self.y = NOTHING
        elif self.__attrs_attrs__['y'].default is not None:
            self.y = self.__attrs_attrs__['y'].default

        # call the factories for all fields that were not given a value through init
        if self.x is NOTHING and isinstance(self.__attrs_attrs__['x'].default, Factory):
            self.x = self.__attrs_attrs__['x'].default.factory(self)
            if self.x is NOTHING:
                raise ValueError("a factory may not return the NOTHING sentinel")

        if self.y is NOTHING and isinstance(self.__attrs_attrs__['y'].default, Factory):
            self.y = self.__attrs_attrs__['y'].default.factory(self)
            if self.y is NOTHING:
                raise ValueError("a factory may not return the NOTHING sentinel")
    
    ...

This has the additional benefit that now I can throw more descriptive error messages during initialization, like:

    ...
    @x.default
    def y_plus_one(self):
        if self.y is NOTHING:
            raise ValueError("either x or y has to be initialized")
        return self.y + 1
    ...
@germaniuss
Copy link
Author

I've written a patch locally and it works, so I'd be willing to send a PR if you're interested.

@hynek hynek added the Feature label Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants