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

how to extend dynamic defaults with super #256

Closed
minrk opened this issue Jul 9, 2016 · 9 comments · Fixed by #332
Closed

how to extend dynamic defaults with super #256

minrk opened this issue Jul 9, 2016 · 9 comments · Fixed by #332
Milestone

Comments

@minrk
Copy link
Member

minrk commented Jul 9, 2016

With traitlets 4.0, it's easy to extend dynamic defaults of parent classes, because the name of the default generator is part of the public API:

class SubClass(Parent):
    def _some_trait_default(self):
        previous_default = super()._trait_default()
        return modified(previous_default)

Now that the name of the method that happens to implement the default generator is no longer a public API, what's the best way to determine what the dynamic default would have been in the parent class, and then extend it in a subclass?

@rmorshea
Copy link
Contributor

rmorshea commented Jul 11, 2016

The 4.0 syntax is still valid (albeit with @default decorating _some_trait_default). Are you saying that because we use @default instead of explicitly requiring the _<trait>_default pattern, we should no longer be accessing underscore names?

If methods named _<trait>_default are meant to be a public API, there's no reason they're respective underscores can't be removed, and an @default decorator added (it'd be tedious, but I wouldn't be opposed to making those edits to Jupyter/Ipython since it's a trivial change).

Outside of that option though, we could define a public method HasTraits.trait_default_generator that iterates over the class mro and accesses the appropriate private generator cache (HasTraits._trait_default_generators).

*note: I can see a potential use case for a metaclass setup step in which BaseDescriptor instances are allowed to touch all the classes which have access to them, not just the ones that "own" them. In this particular case it would means DefaultEventHandler instances could install the full range of default generators on a class so that iterating over the mro wouldn't be necessary when finding default generators. This would create more overhead when importing traitlets based modules however it would greatly simplify TraitType._dynamic_default_callable.

@rmorshea
Copy link
Contributor

rmorshea commented Jul 11, 2016

To add to this question though:

There is no easy way to apply modifications to the value of TraitType.default_value or TraitType.make_dynamic_default inside a default generator because trait instances are never passed to default generators.

This could potentially be answered in #230 or #251 by passing a bunch to default generators (checking the function signature before doing so to keep things backward compatible)

@minrk
Copy link
Member Author

minrk commented Jul 25, 2016

Are you saying that because we use @default instead of explicitly requiring the __default pattern, we should no longer be accessing underscore names?

No, I mean there's no way to answer the question "what would the dynamic default value have been in my parent class," which is used often in subclasses to modify defaults.

Let's say I have the following:

class Parent(HasTraits):
    some_trait = List(Unicode())
    def _some_trait_default(self):
        return [ os.getcwd() ]

In 4.0 and earlier, I could reliably extend this class to add to the default in the child:

class Child(Parent):
    def _some_trait_default(self):
        return super(). _some_trait_default() + ['/']

As far as I can tell, this pattern is now impossible via public APIs, because I can no longer determine what the super.default would have been without reading private APIs in the source code of the parent class. We need a public way of asking "what would my parent's default value have been?".

@rmorshea
Copy link
Contributor

So you saying that because there's not a naming scheme for default generators you can have a situation like this:

class Parent(HasTraits):
    i = Int()
    @default('i')
    def _dgen1(self):
        return 1

class Subclass(Parent):
    @default('i')
    def _dgen2(self):
        # raises attribute error
        return super(Subclass)._dgen2() + 1

Where super is only reliable when new default generators override the parent method instead of defining new ones.

@minrk
Copy link
Member Author

minrk commented Jul 26, 2016

The main problem is that the name of _dgen1 is a free, undefined private API that subclass can't rely on, which it wasn't before, and there isn't a way to discover it.

@rmorshea
Copy link
Contributor

rmorshea commented Oct 3, 2016

I thought about this again today, and while you can reliably call _<trait>_default with traitlets 4.0, the fact that you can call this method doesn't mean it was ever actually used to make a default value:

class A(HasTraits):

    i = Int()

    def _i_default(self):
        return 1

class B(A):

    i = Int(2)

b = B()
true_default = b.i
false_default = b._i_default()

print(true_default)
print(false_default)

As it turns out, _<trait>_default is NOT a reliable way to discover a trait's default value.

@rmorshea
Copy link
Contributor

rmorshea commented Oct 3, 2016

@minrk what's you're take on the above issue?

IMO this is an insoluble problem given our current heuristics.

@minrk
Copy link
Member Author

minrk commented Oct 4, 2016

Good point, though in practice it worked. We've never had a true, public mechanism for asking the 'what would my default have been'. It would be nice to come up with one.

@rmorshea
Copy link
Contributor

rmorshea commented Oct 5, 2016

I've been working on some unobtrusive changes that would make it possible to introspect HasTraits._trait_default_generators - in other words, all possible avenues for creating default values would ultimately register a callable to that dictionary. Thus, to find the default value of a trait, you would do this:

HasTraits._trait_default_generators[trait](instance)

Likewise, super would allow you introspect that parent class as well:

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

Successfully merging a pull request may close this issue.

3 participants