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

Add an attribute with the default value to Setting #37

Open
ziima opened this issue Sep 19, 2018 · 5 comments
Open

Add an attribute with the default value to Setting #37

ziima opened this issue Sep 19, 2018 · 5 comments

Comments

@ziima
Copy link
Collaborator

ziima commented Sep 19, 2018

Several Setting subclasses override their __init__ just to provide a different default value. We should add an attribute which would replace the need to override the __init__ just in this case.

@pawamoy
Copy link
Owner

pawamoy commented Sep 19, 2018

Can you show an example? I'm not sure to understand how you would do this.

@ziima
Copy link
Collaborator Author

ziima commented Sep 19, 2018

I think about something like:

class Settings(object):
    default_default_value = None

    def __init__(self, ..., default=NOT_PROVIDED, ...):

    @property
    def default_value(self):
        if self.default == NOT_PROVIDED:
            return self.default_default_value  # Give or take the callable
        ... # Current implementation

class DictSettings(Settings):
   default_default_value = dict

I'm in a trouble with names, since both default and default_value are already used. So those are definitely temporary.

@pawamoy
Copy link
Owner

pawamoy commented Sep 19, 2018

OK I see, thanks!

What about this:

SENTINEL = object()

class Settings(object):
    default = None

    def __init__(self, ..., default=SENTINEL, ...):
        ...
        if default != SENTINEL:
            self.default = default
        # otherwise self.default is already set with the class attribute
        ...

    # default_value property remains unchanged

class DictSettings(Settings):
    default = dict

Though I'm not sure it's good practice.

By the way it could be applied to other arguments if needed.

@ziima
Copy link
Collaborator Author

ziima commented Sep 20, 2018

I don't like the class attribute to be change to instance attribute when passed as argument to __init__. I'm more in favor of having those two separated.

Other arguments might benefit from this approach as well, but I'd rather start with one ;-)

@pawamoy
Copy link
Owner

pawamoy commented Sep 22, 2018

OK for separating them! And I honestly can't find a better name than default_default_value. Unless if we use uppercase, like DEFAULT or DEFAULT_VALUE, but it's a bit ugly.

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

2 participants