too many classes in Qt

[Carreau]

@minrk opened ipython/ipython#7325

We have too many classes and subclasses in the qtconsole code. Right now, we have:

RichIPythonWidget inherits from IPythonWidget inherits from FrontendWidget inherits from (HistoryConsoleWidget, BaseFrontendMixin) inherits from ConsoleWidget, and RichIPythonWidget is the only class we actually use. None of the intermediate classes serve any use.

I think a lot of this is abstraction for abstraction's sake, and we can get rid of most or all of these classes, leaving the one and only real final IPythonWidget as the implementation of all of these things, especially the BaseFrontendMixin, which is a Mixin only mixed into one class. The ConsoleWidget makes a certain amount of sense, since it contains a lot of generic logic, but I think at least, BaseFrontendMixin, FrontendWidget, IPythonWidget, and RichIPythonWidget should probably be consolidated into a single class.

[Carreau]

@takluyver commented

Ping @ccordoba12 @epatters @rkern @jonathanrocher @jdmarch - do you directly use any of the Qt console *Widget classes besides RichIPythonWidget? We'd like to collapse the inheritance hierarchy.

[Carreau]

@Carreau commented

Can we do that after the codesplit ?

[Carreau]

@takluyver commented

Certainly, that's one thing where it doesn't make much difference, because all of that will be in one repo post-split.

[Carreau]

@ccordoba12 commented

Nop, we just use RichIPythonWidget. However, given that that's sort of public API, could its name be preserved?

Edit: I think the other classes are there to create more generic interfaces (like a Python only frontend) but given that there haven't been any movement in that direction, I agree with the simplification.

[Carreau]

@jdmarch commented

We also use FrontendWidget. Pinging @pankajp for further evaluation.

[Carreau]

@minrk commented

Yup, definitely appropriate for after the code split. I was just working on the qt code, and it was bugging me, so I opened an issue. There's a chance that it will bug me enough to just merge IPythonWidget and RichIPythonWidget between now and release, since the IPython-aware widget that doesn't understand anything but plain text intermediate doesn't make a lot of sense. But I don't think any of this is needed for 3.0.

[Carreau]

@minrk commented

@jdmarch can you point to code where you use FrontendWidget? Or at least describe how it's used?

I think getting rid of the small mixin that's mixed into exactly one class should be simple enough. It would leave FrontendWidget unchanged.

[Carreau]

@pankajp commented

@jdmarch We don't really "use" FrontendWidget, its use in code is only due to mapping the IPython's widget hierarchy into pyface. Simplifying the widget class hierarchy in ipython would be good, and probably a good time to get rid of some of our code as well.

[Carreau]

@minrk commented

Based on my understanding, I would probably do the following:

• Merge HistoryConsoleWidget into ConsoleWidget. I don't think a ConsoleWidget without history makes a great deal of sense.
• Merge BaseFrontendMixin into FrontendWidget, the only class into which the mixin is mixed.
• Merge RichIPythonWidget into IPythonWidget. A plain-text-only IPythonWidget doesn't make much sense.
• Possibly merge FrontendWidget into IPythonWidget, depending on Enthought feedback about how useful a FrontendWidget is, without the IPython part.

That would bring the situation from 6 classes down to 3 or even 2, which I think would be a lot more manageable. One of the most confusing parts of working on the Qt code is the super() hierarchy of some methods, where 'simple' cases are handled in a parent class, but complex cases of the same event are defined in a child.

[Carreau]

@jdmarch commented

@minrk I think it's clear from @pankajp's response, but to be explicit: as far as we are concerned: go for it