ivoflipse / Pawlabeling

Tool for processing and analyzing pressure measurements
Other
18 stars 1 forks source link

Move all the data handling to model classes #33

Closed ivoflipse closed 10 years ago

ivoflipse commented 11 years ago

Currently all the data is kept internally in each separate widget. This means if I load the results in the analysis widget and want to display them in the different results widgets, I have to copy the data in every widget.

Worse, any results I calculate, like the average paw, have to be recalculated for each different result.

Given that the processing and analysis widgets have different requirements, like keeping track of the measurements where the contacts come from, I might make different models for the different widgets.

ivoflipse commented 11 years ago

I've moved all the data handling/processing from the processing widget to a model class. The only data stored in the processing widget is needed to update the contact tree, if I were really doing things the Qt way I would probably put everything a QTreeModel or something, but I don't think I really need it.

Basically the processing widget now handles the GUI and any actions the users performs.

Now on to the analysis widget

ivoflipse commented 11 years ago

While working on making a model for the analysis widget, I realized I was duplicating a lot of code. So I've decided to refactor the code a bit more and moved everything to a base model.py that's going to be shared between all widgets.

Because I've added the model to the MainWindow, all communication must go through pubsub, because there's too many layers of nesting for other message passing

ivoflipse commented 11 years ago

There are some interesting 'issues' with sharing the model between processing and analysis. Perhaps I should create separate functions for updating the analysis widgets and the processing widgets, such that we don't do additional work even though its not even visible (like tracking contacts while you're analyzing already labeled results) and because the analysis widgets simply want a different type of results.

Worse, currently if I change a frame in the analysis, all tabs start updating their frames, which is rather unwanted if they are not visible.

ivoflipse commented 11 years ago

I think I've fixed most problems related to the model, so this branch is almost good to merge back in. However, I'm considering refactoring my analysis part, because there's so much overlap between the different results, it seems a bit wasteful to have everything be a separate class

ivoflipse commented 10 years ago

Well my model certainly does most if not all of the heavy lifting. I still have some bits and pieces I would like to remove from the views, so they can't mess things up because I forgot to delete them.

Now I've actually reached the point where my model.py class has gotten so large, its probably time to refactor it into smaller pieces. Possibly creating similar modules like contactmodel, that focuses solely on handling the interaction with contacts.

I'm also considering making two 'versions' of model.py, where one deals with the in coming UI messages and tracks the state of the program (like which subject is selected), whereas the other is more like a package, which does the actual PyTables interactions or calculations, but is stateless and doesn't care that there's a GUI or some other code involved. This should help make the code more reusable outside of the application, because now almost every function exclusively returns data through pubsub, making it useless without it.

ivoflipse commented 10 years ago

My model.py is getting very long and while certain functions naturally go together, there are others that would do better when they were isolated to the class they belong to.

For example, calculating average results over all contacts in a measurement belongs in a measurementmodel and likewise for sessions.

ivoflipse commented 10 years ago

I've moved all the code to separate model classes, obviously that's a bit over the top. But I'm going to try and refactor it such that the model only stores the state and notifies the rest of the application. It should leave the calculations and such to the other models.

If models need certain information from each other, I'll have to think up a way to share this information cleanly. Preferably without having the other models resort to pubsub.

ivoflipse commented 10 years ago

I've broken everything into little pieces, where the models themselves hold very little state and model.py basically calls them whever it wants something done. I could change the responsibility once more to have the models hold the state and simply return a reference from model.py to move around, but I think I'll leave it like this for now. Case closed.

ivoflipse commented 10 years ago

I think I took it a little bit too far, while the Session/Measurement etc classes make some sense, the Sessions/Measurements mostly do not. They're supposed to be a way to manage the classes they encapsulate, but given how little state they have, they might as well be functions.

So a better way of organizing it would be to have the model remember all the state, keep a list or dictionary of all class instances and have those class instances remember the state. Then update_average for example, should be a function of the Session class, which expects you to pass it its contacts (if it doesn't have them already) and calculate the average over them and keeps track of them.

That way variables like self.session or self.measurement could be replaced with self.sessions[self.session_id] or self.measurements[self.measurement_id], so you know you always get the right version (if you set the IDs correctly), but you can't get situations where the id was updated, but the self.session object didn't

ivoflipse commented 10 years ago

I've pretty much moved all data handling to the models. While I probably still took it a bit too far and the way I calculate averages is still a bit brittle, at least everything gets its state from the model.