Xi-CAM / Xi-cam.gui

0 stars 9 forks source link

New stream arg in tabview conflicts with sub classes of ImageView #44

Open dylanmcreynolds opened 4 years ago

dylanmcreynolds commented 4 years ago

This was reported by Peter Ercias. The xi-cam.NCEM plugin cannot instatiate because of the stream kwarg introduced in cea06d189da0fc12130c55aeb7d2a5d0a3b0e2f8.

When the DunImageView instantiates, it complains that stream is unexpected.

File "c:\users\supervisor\utilities\xicam2\xi-cam.gui\xicam\gui\widgets\tabview.py", line 89, in dataChanged
    newwidget = self.widgetcls(itemdata, stream=self.stream, field=self.field, **self.kwargs)
   File "c:\users\supervisor\utilities\xicam2\xi-cam.ncem\xicam\NCEM\widgets\NCEMViewerPlugin.py", line 22, in __init__
    super(NCEMViewerPlugin, self).__init__(**kwargs)
   File "c:\users\supervisor\utilities\xicam2\xi-cam.gui\xicam\gui\widgets\dynimageview.py", line 7, in __init__
    super(DynImageView, self).__init__(*args, **kwargs)
 TypeError: __init__() got an unexpected keyword argument 'stream'
ercius commented 4 years ago

A similar problem occurred with the metadataviewer. @ronpandolfi fixed it by removing the excludedkeys keyword before the super() call.

https://github.com/synchrotrons/Xi-cam.gui/blob/4274cffb277774b01608bbf82f7be1d2f4c3a255/xicam/gui/widgets/metadataview.py#L39

ercius commented 4 years ago

My solution is to change the dynimageviewer.__init__ as follows:

class DynImageView(ImageView):
    def __init__(self, *args, **kwargs):
        if 'stream' in kwargs:
            del kwargs['stream']
        super(DynImageView, self).__init__(*args, **kwargs) 

The plugin continues to work in this case

ihumphrey commented 4 years ago

That should work for NCEMViewerPlugin, but it I'm not sure it will work for FFTViewerPlugin and FourDImageView, since they don't inherit DynImageView. I'm thinking about what we can do about this (see notes that will follow).

ihumphrey commented 4 years ago

Notes:

When instantiating the widgetcls passed to TabView, we pass the itemdata (which can be a NonDBHeader or BlueskyRun), a stream, a field, and any additional kwargs passed into TabView:

# TabView constructor for references
class TabView(QTabWidget):
    def __init__(
            self,
            catalogmodel: QStandardItemModel = None,
            selectionmodel: QItemSelectionModel = None,
            widgetcls=None,
            stream=None,
            field=None,
            bindings: List[tuple] = [],
            **kwargs,
    ):
# ...
# tabview.py:89
newwidget = self.widgetcls(itemdata, stream=self.stream, field=self.field, **self.kwargs)

If the widgetcls does not capture stream (or field) in its constructor, these will be passed as kwargs in the inheritance change. This can (as @ercius wrote above) cause a base class to fail to initialize due to unexpected arguments.

Here are some design questions I have regarding this:

Example of this issue occurring for `NCEMViewerPlugin`, `FFTViewerPlugin`, `FourDImageView`. ```python class NCEMPlugin(GUIPlugin): # .... def __init__(self): # .... self.rawview = TabView(self.headermodel, self.selectionmodel, widgets.NCEMViewerPlugin, 'primary') self.fftview = TabView(self.headermodel, self.selectionmodel, widgets.FFTViewerPlugin, 'primary') self.fourDview = TabView(self.headermodel, self.selectionmodel, widgets.FourDImageView, 'primary') ``` ```python class NCEMViewerPlugin(DynImageView, QWidgetPlugin): def __init__(self, header: NonDBHeader = None, field: str = 'primary', toolbar: QToolBar = None, *args, **kwargs): # ... super(NCEMViewerPlugin, self).__init__(**kwargs) ``` ```python class DynImageView(ImageView): def __init__(self, *args, **kwargs): super(DynImageView, self).__init__(*args, **kwargs) ``` ```python class ImageView(QtGui.QtWidget): def __init__(self, parent=None, name="ImageView", view=None, imageItem=None, *args): ``` ```python class QWidgetPlugin(QWidget, IPlugin): isSingleton = False # no __init__, implicit QWidget construction ```
ercius commented 4 years ago

For now Im going to fix this in my plugins by adding stream = 'primary' to the init class of the plugin.