Cannot instantiate a subcommand if it inherits the same baseclass with a parent-cmd
- Langage dominant
- Python
- Étoiles
- 653
- Forks
- 217
- Merge moyen
- 2 j 21 h
- PR mergées (30 j)
- 2
Description
## 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.
Guide de contribution
Ouvrir le guide de contribution
Évaluation
Cette issue n'a pas encore été évaluée.