ipython / ipython/traitlets

Cannot instantiate a subcommand if it inherits the same baseclass with a parent-cmd

オープン
#474 コメント 9 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
Python
スター
653
フォーク
217
平均マージ
2日 21時間
マージ済み PR(30日)
2

説明

## Problem
As currently implemented (till 5.x), subapps are actually constructed in `ìnitialize_subcommands()` by invoking `SingletonConfigurable.instance()`. And that makes sense, to make it easily to locate a command instance, regardless of its position in the "command chain".

But since this `SingletonConfigurable.instance()` class-method store the new instance to **all baseclasses**, this leads easily to clashes in case 2 subcommands share the same baseclass (which is always the case, since `Application` is a common parent).

Actually from within IPython you cannot even use any sub-command at all:
```python
>>> class App(Application):
... pass
...
>>> class Sub(Application):
... pass
...
>>> app = App(subcommands={'sub': (Sub, 'cmd title')})
>>> sub.initialize(['sub'])
Traceback (most recent call last):
...
MultipleInstanceError: An incompatible sibling of 'Sub' is already instanciated as singleton: TerminalIPythonApp
```

To signify the problem, here is a trivial example that fails even in pure python REPL:
```python
>>> class Sub1(Base):
... pass
...
>>> class Sub2(Base):
... pass
...
>>> app = Application(subcommands={'sub1':(Sub1, 'help'), 'sub2': (Sub2, 'help')})

>>> app.initialize(['sub1'])
>>> app.subapp
<__main__.Sub1 object at 0x000001C2D5D77C50>

>>> app.initialize(['sub2'])
Traceback (most recent call last):
...
traitlets.config.configurable.MultipleInstanceError: An incompatible sibling of 'Sub2' is already instanciated as singleton: Sub1
```
Anothe side-effect is that test-cases on sub-commands require spurious `clear_instances()` invocations to work (see `test_application.test_subcommands_instanciation()`).
## Possible Mitigations
Either:
1. Modify `initialize_subcommands()` not to use `Singleton.instance()` (or use a new method instead?), or
2. modify `Singleton.instance()` and `clear_instances()` not to set/clear newly instanciated app to all `mro()` classe-attributes.

I would prefer (2), assuming there is not some blocker reason for why `instance()` behaves like that.

Of course it might be that my understanding is totally broken here.

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。