ProtProtocols / IsoProt

Protocol of the analysis of iTRAQ/TMT proteomics data including quantification, statistical analysis and maybe clustering
https://protprotocols.github.io
11 stars 4 forks source link

Separation of GUI, Data and Searching #25

Closed gvinterhalter closed 6 years ago

gvinterhalter commented 6 years ago

I believe that the code can be improved by separating the GUI from the data it represents and the logic of Searching the task. Currently, I'm not sure if separating the data from GUI is possible but achieving some sort of model-view-wildcard pattern wold be nice. What I wold hope to get is:

  1. The separation wold lead to more manageable code with clear task separation.
  2. After it matures the GUI stuff and helper functions etc. can be moved to ProtProtocols for reuse.

I can try and give it a shot if you guys are OK with it?

jgriss commented 6 years ago

Hi @gvinterhalter !

As you probably saw I've now move the python code in a separate file. Now it can be edited in a "normal" Python IDE which makes the development a lot easier.

Your suggestion is very good! As you might have seen, I already tried to put the main GUI elements in separate classes. The rest of the code is still very messy since I was figuring how everything works while writing the code.

If you have some time to spare it would be great if you could try to clean up the rest!

I've just committed the latest version of the Scripts/search_tool.ipy file where I already started to clean up bits and pieces.

Cheers, Johannes

gvinterhalter commented 6 years ago

Hello @jgriss ! I was exploring ipywidgets and traitlets to see what is possible.

I tried to separate the model from the view(widgets). I do like the idea of having a Model class inheriting traitlets.HasTraits and then using traitlets.link to connect to widgets contained in a separate View class. Linking and even view creation can be automated but it's still a bit awkward and might be a bit too complicated. There are also problems. For example, I can't easily do validation of values as they can be changed at 2 places.

It wold seem the best option is not to have this separation (leave stuff as they are). The model and it's view is defined in one class that will run the search but the search is agnostic to the UI, it get's a dict of parameters or something similar.

gvinterhalter commented 6 years ago

Just to clarify what I was going for before...

import ipywidgets as wig
import traitlets as trt
from IPython.display import display

class BaseModel(trt.HasTraits):
    def __repr__(self):
        '''Represent it as a dict of values'''
        return str( {k:getattr(self, k) for k in self.trait_names()} )

    def save_settings(self):   pass
    def laod_settigns(self):   pass

class BaseView():

    def setLayout(self, root_widget):
        self._layout = root_widget

    def get_widgets(self):
        '''Return a dict of of widgets. '''
        return {k:v for k, v in self.__dict__.items() if isinstance(v, wig.Widget) and k[0]!='_' }

    def asign_model(self, m, directional_link=False):
        self._model = m
        self._links = []

        link_function = trt.link if not directional_link else trt.directional_link

        for w_name, w in self.get_widgets().items():
            # set the value before the link!
            w.value = getattr(m, w_name) 
            # automatically set description, this can be put somewhere else
            w.description =  w_name.replace('_', ' ').capitalize() 
            #link and save the link
            self._links.append( link_function((w, 'value'), (m, w_name)) )

        return self

    def _repr_html_(self):
        from IPython.display import display

        # if layout exists, display it
        if hasattr(self, '_layout'):
            return display(self._layout)

        # else, just display what we have
        widget_list = self.get_widgets().values()
        return display( *widget_list ) 

############ Use case ################

class TaskParam(BaseModel): 
    check_me   = trt.Bool(True)
    some_text  = trt.Unicode()
    some_int   = trt.Int(5)
    some_float = trt.Float(3.15)    

class View(BaseView):

    def __init__(self):    
        # We can probably make this creation autoamtic or whatever but then we lose flexibility
        self.check_me   = wig.Checkbox()
        self.some_text  = wig.Textarea()
        self.some_int   = wig.IntSlider()
        self.some_float = wig.FloatSlider()

        self.makeLayout()        

    def makeLayout(self):
        '''Make some layout'''

        layout = wig.VBox([self.check_me, self.some_text, self.some_int, self.some_float])
        self.setLayout(layout)

task_parameters = TaskParam()
task_view = View().asign_model(task_parameters)
display(task_view)

task_parameters.some_text = 'hello world' # change from model side
gvinterhalter commented 6 years ago

I was tinkering with the idea of automating the UI and stuff. It's not really there jet, probably it's useless. But if you want, take a look. (This stuff requires careful design... ) ui.py.zip

You can try it by creating SearchUI

jgriss commented 6 years ago

Hi! I like your idea a lot! But we also need to focus on getting a working version ready.

Therefore, I'd suggest to follow your previous suggestion: leave the code as it is now. Make the search code agnostic to the UI and create some defined variable that's available to other code running in the notebook. Thereby, other cells can then use these settings to adapt their behavior.

In parallel, we can think about whether we can create a proper framework to solve this. At the moment I'm not fully convinced that it's worth the effort. But I agree that it would be great to have something like this.

Does this make sense?

veitveit commented 6 years ago

I also like the idea much. The problem is that we do not yet know what other protocols will be and therefore are not clear about their necessities. For instance, do we want to restrict to workflows that can be executed via bash and scripts?

In general (if we go along that line), a more structured representation of all parameters (file or python structure) would be helpful, and could work as an interface between GUI (with that I mean the widgets) and the calls of the tools.

gvinterhalter commented 6 years ago

@jgriss I agree with everything you said.

@veitveit Curently all parameters are stored as .json, but I was thinking to recomend .toml (intro) for storing (configuring) search parameters. You can edit them by hand pretty easily and there are lot's of parses for Python and R. You can also have a notebook UI that will generate .toml and then bash can take over. I'm just not sure about bash support but I assume it can get help from python or R.

For example, Metamorpheus PTM Discovery tool is using it to store search parameters. Here is an example Task5SearchExample.toml.zip

jgriss commented 6 years ago

Hi @gvinterhalter and @veitveit,

Since I believe that this is getting a larger project, I created a dedicated "project" to collect all related issues (Python Gui / Workflow Tool Set).

There we can collect all ideas as issues and simply add the ones we think should be dealt with first to the "v0.1" milestone.

Does that sound OK?

jgriss commented 6 years ago

Alternatively, we can also add an additional column for "planned" features and only move the ones we want to address first in the "ToDo" column.

jgriss commented 6 years ago

As this is quite similar to #28 and we've now started to add a module structure for the python code I believe that we can close this issue and add new issues in the Python Gui / Workflow Tool Set project.