Don't create default values for Instance until required

[takluyver]

This is an alternative to #23 and #28

There is already a mechanism for dynamic default values with _foo_default methods, which are not called unless needed. This makes the default values for Instance traitlets use that same mechanism, avoiding instantiating classes unless those traitlets are explicitly requested without being assigned first.

Along the way, two other changes to behaviour, and hence to tests, came in. Dict traitlets are based on Instance, so their behaviour changes in the same way. And a Type() traitlet with no class specified now gets a default value of object instead of None, because the former is valid when allow_none is False.

It was also possible to clean up some parts of the machinery, leading to a reduction in total line count.

[takluyver]

N.B. Previously _trait_dyn_inits was a mapping of trait name to method name on the object. Now it can contain either a method name or a callable. It's private, and this is the only place we use values from it, I believe.

[SylvainCorlay]

Switching behaviors based on if isinstance statements is not very object-oriented IMO.

[SylvainCorlay]

The problem with calling _setup_dynamic_initializer in instance_init is that instance_init is called for traits that are held by other traits (such as Int in List(Int())), and you probably don't want that.

That is why I created the top_instance_init function in #28 which is not called for subtraits.

[takluyver]

I did notice that that was odd, but it doesn't actually cause any problems - self.name is None in those traits, so it just sets a None key in the object dictionaries which goes unused. I'll look into cleaning that up in a separate PR.

[SylvainCorlay]

In Container, the subtrait name is set to element.

[takluyver]

With the last commit, traits that are only used for validation (inside Containers or Unions) do not get a name set, and do not look at methods or the trait dictionaries on the HasTraits instance.

If I were designing this from scratch, I'd try to more cleanly separate the validation parts of traitlets from the descriptor parts (notification, _foo_default methods on the owner, etc.). But I'm not.

[SylvainCorlay]

• With that change TraitType.name becomes implicitly a switch to enable / disable the rest of instance_init.
• Besides, _trait_dyn_inits holding either a callable or a string makes the code more complicated to follow.
• I strongly dislike having isinstance calls when we are not doing metaclass things.

I prefer having clearly two kinds of instance_init, one that is top-level only (top_instance_init) and one that is used for sub-traits (instance_init) clarifies things.

[takluyver]

1. It doesn't disable things subclasses do, only the logic to set up default values. And it's not an implicit switch - that code uses self.name to look several things up on the instance. Given how self.name is used, it only makes sense for traitlets attached directly to an instance.
2. Not by very much, because that dictionary is only accessed in one place.
3. We use lots of isinstance calls, because practicality beats purity. But I'll look into achieving the same thing without isinstance.

[takluyver]

_trait_dyn_inits and the isinstance check for values from it are now gone.

[SylvainCorlay]

@takluyver thanks, I like the new version much more.
And 👍 for all the docstring corrections.

[takluyver]

Great. I'd like @minrk to have a look over this when he has a chance. Then hopefully we can get traitlets 4.0 out, and start preparing the next level of things.

[minrk]

I'll make sure to have a look tomorrow.

[minrk]

deprecation warning on the deprecated method?

[minrk]

I made one comment about deprecation, but I think this looks sensible. Do we close #28 if we merge this?

[SylvainCorlay]

Sure, no problem for me.

[takluyver]

Added a warning.

I wonder about also deprecating get_default_value(), as it's now just self.default_value in all cases that we provide. Are there any traitlets for widgets or anything where get_default_value() does something else?

[minrk]

I'm not aware of any. Should we merge here, and let that question be another issue?

[takluyver]

Sounds like a plan. Created issue #32 for that deprecation issue.

[minrk]

Okay, then.

[SylvainCorlay]

@takluyver This might have broken the widgets (cross validation of the slider values). See jupyter-widgets/ipywidgets#52.

[takluyver]

Looks like the issue is that max tries to validate against min, but looking up the default value of min triggers a validation of that, which checks max, and so ad infinitum. I'm guessing if #33 is merged and widgets updated to use it, the error will go away.

However, I think I know the change that caused this. Before, static default values were transferred into obj._trait_values at instance init. I thought that was unnecessary, so I removed it. I'll make another PR which should fix it.