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 parent to Configurable #3430

Merged
merged 6 commits into from Jul 1, 2013
Merged

add parent to Configurable #3430

merged 6 commits into from Jul 1, 2013

Conversation

minrk
Copy link
Member

@minrk minrk commented Jun 14, 2013

this adds the notion of a parent and member config, so the config:

c.Foo.Bar.attr = value

will only set Bar.attr = value for Bar instances which are members of Foo instances. The mechanism for doing this is

f = Foo(config=cfg) f.b = Bar(parent=f)

This Instance config has higher priority than plain class config for Bar, but still lower priority than direct keyword arg trait assignment.

The main implication this has is to change the standard creation of descendants:

self.bar = Bar(config=self.config)

into a direct parent expression

self.bar = Bar(parent=self)

This also means that most Configurables will actually have a handle on their parent object.

It also means that most IPython configurables have a handle on their parent object.
If this is undesirable, we can construct the name list at init, without holding onto the key.

@minrk
Copy link
Member Author

minrk commented Jun 14, 2013

still a few tests to fix

@ellisonbg
Copy link
Member

Does this work deeper than two levels like Foo.Bar.Bam.option?

@ellisonbg
Copy link
Member

Does each child still have its own copy of the config data, or does it refer to the parents?

@minrk
Copy link
Member Author

minrk commented Jun 15, 2013

Does this work deeper than two levels like Foo.Bar.Bam.option?

I don't think so, but perhaps it should. There is another question here - should Foo.Bam configure Bam in the Foo.Bar.Bam case (i.e. any descendent)?

Does each child still have its own copy of the config data, or does it refer to the parents?

There is no change in who has what data or how many copies. The only change is the optional addition of parent, which is a handle on the parent object instance.

@ellisonbg
Copy link
Member

I see a failure in IPython.kernel.tests.test_multikernelmanager.TestKernelManager.

@ellisonbg
Copy link
Member

If it is not too difficult to make it work deeper than 2 levels, I think we should do that.

In terms of the skipping of descendants, I could imagine usage cases where you would want both behaviors. But maybe we should keep it simple for now and require all classes to be listed. If we run into issues with that approach later, we can change it. We have survived for a few years without this capability so I am not sure exactly how it will be used.

@fperez
Copy link
Member

fperez commented Jun 28, 2013

Hey @minrk, it needs a rebase now, and it looks like a bit of work... Do you think you want/have bandwidth to finish this one in the next week?

@minrk
Copy link
Member Author

minrk commented Jun 28, 2013

Sure, should be easy enough.

this adds the notion of a parent and member config,
so the config:

    c.Foo.Bar.attr = value

will only set `Bar.attr = value` for `Bar` instances
which are members of `Foo` instances.
The mechanism for doing this is

```python
f = Foo(config=cfg)
f.b = Bar(parent=f)
```

This Instance config has higher priority than plain class config for Bar,
but still lower priority than direct keyword arg trait assignment.

The main implication this has is to change the standard creation of descendants:

```python
self.bar = Bar(config=self.config)
```

into a direct parent expression

```python
self.bar = Bar(parent=self)
```

This also means that most Configurables will actually have a handle on their parent object.
instead of `config=self.config`

only real effective change: IPythonKernelApp.parent has been renamed to IPKernelApp.parent_handle.
now Foo.Bar.Baz can affect a Baz whose parent is Bar whose parent is Foo.

More specificity = higher priority, so Foo.Bar.Baz > Bar.Baz > Baz.
Configurables often resolve to zero for some reason
@minrk
Copy link
Member Author

minrk commented Jun 29, 2013

rebased, and Foo.Bar.Bam should work now.

I also fixed the issue in the MKM test

@ellisonbg
Copy link
Member

This looks good, merging.

ellisonbg added a commit that referenced this pull request Jul 1, 2013
@ellisonbg ellisonbg merged commit 3bde924 into ipython:master Jul 1, 2013
@minrk minrk deleted the configinstance branch March 31, 2014 23:36
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
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 this pull request may close these issues.

None yet

3 participants