Cannot instantiate a subcommand if it inherits the same baseclass with a parent-cmd
- Lenguaje dominante
- Python
- Estrellas
- 653
- Forks
- 217
- Merge medio
- 2 d 21 h
- PR fusionados (30 d)
- 2
Descripción
## 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.
Guía de contribución
Evaluación
Este issue todavía no se ha evaluado.