Implement viewer for import tomograms #3
Conversation
tomo/viewers/viewer_tomograms.py
Outdated
| import pyworkflow.protocol.params as params | ||
| from pyworkflow.em.convert import ImageHandler | ||
| from pyworkflow.viewer import DESKTOP_TKINTER, WEB_DJANGO, ProtocolViewer | ||
| from pyworkflow.em.viewers.viewer_chimera import Chimera, ChimeraView |
There was a problem hiding this comment.
Import should be done from submodule pyworkflow.em.viewers, viewers_chimera is an implementation file and should not be imported directly.
tomo/viewers/viewer_tomograms.py
Outdated
| from pyworkflow.viewer import DESKTOP_TKINTER, WEB_DJANGO, ProtocolViewer | ||
| from pyworkflow.em.viewers.viewer_chimera import Chimera, ChimeraView | ||
|
|
||
| from tomo.protocols.protocol_import_tomograms import ProtImportTomograms |
There was a problem hiding this comment.
Same here, import from tomo.protocols
tomo/viewers/viewer_tomograms.py
Outdated
| TOMOGRAM_CHIMERA = 0 | ||
|
|
||
|
|
||
| class viewerProtImportTomograms(ProtocolViewer): |
There was a problem hiding this comment.
Class names should start with capital letter
tomo/viewers/viewer_tomograms.py
Outdated
| return [self.objectView(setOfTomograms)] | ||
|
|
||
|
|
||
| def errorWindow(tkParent, msg): |
There was a problem hiding this comment.
I think there are error messages in the base classes of Viewers (or ProtocolViewers), so I think this function is unnecessary here.
|
@delarosatrevin I think I have taken care of all your comments. Let me know otherwise... Thanks! |
tomo/viewers/viewer_tomograms.py
Outdated
| from pyworkflow.viewer import DESKTOP_TKINTER, WEB_DJANGO, ProtocolViewer, MessageView, MSG_ERROR | ||
| import pyworkflow.em.viewers as viewers | ||
|
|
||
| from tomo.protocols import protocol_import_tomograms |
There was a problem hiding this comment.
Still here you are referring to "internal" implementation file 'protocol_import_tomograms'. This is not a good practice since this file can change, or merged...the class definition should be available from tomo.protocols (defined in its init.py file), so it should be something like this:
from tomo.protocols import ProtImportTomograms # so you don't really need to know where the class is implemented
tomo/viewers/viewer_tomograms.py
Outdated
|
|
||
| import pyworkflow.protocol.params as params | ||
| from pyworkflow.em.convert import ImageHandler | ||
| from pyworkflow.viewer import DESKTOP_TKINTER, WEB_DJANGO, ProtocolViewer, MessageView, MSG_ERROR |
There was a problem hiding this comment.
Please check line length of max 80, as stated in PEP8
| } | ||
|
|
||
| def _validate(self): | ||
| if (self.displayTomo == TOMOGRAM_CHIMERA |
There was a problem hiding this comment.
There is a plugin specific to Chimera, have you checked if some of this functionality is already there?
|
I have added some extra comments. |
For now, this is just mirroring the viewer volumes behaviour. As discussed with @delarosatrevin we will have to find a more intelligent approach as this viewer won't be able to handle regular-size tomograms (a few GBs). This is just a quick solution for the moment.
We have just discussed to implement the viewer per data type instead of per protocol. However, if we do it so the viewer_volume will be triggered instead of the viewer for Tomograms or Set of Tomograms.