glasgowcompbio / vimms

A programmable and modular LC/MS simulator in Python
MIT License
19 stars 6 forks source link

Reorganise scan processing within controllers #15

Closed vinnydavies closed 2 years ago

vinnydavies commented 4 years ago

Reorganise scan processing within controllers to remove duplications. Probably easiest to wait for #8 to be completed first

vinnydavies commented 3 years ago

Probably best to not mess with this old code anymore, so not going to be updating the process_scan code in the base controllers (TopNController). Instead there is a new controller called GridController from which future controllers can be built off which has a more organised version of process_scan. Assuming future controllers are built from here then that sorts this issue

joewandy commented 3 years ago

How's the process_scan inside GridController more organised in comparison to the old one, may I ask?

vinnydavies commented 3 years ago

@mcbrider5002 cleaned it up a little bit. Can clean it up more if you want, could be worthwhile. Suggest you clean that controller up rather than the original base controllers, so we dont mess up any of the old controllers

mcbrider5002 commented 3 years ago

Scheduling MS1 and MS2 scans was abstracted out a bit, so any class inheriting from GridController and needing to implement _process_scan shouldn't need to duplicate the scan scheduling code. (Also, there's no duplication in the multiple cases of scheduling MS1.) In theory you could move these abstractions further up the class hierarchy (Vinny does not want to break things by doing this, but should be able to rearrange things internally while still exposing the same interface). There's still a lot of duplication within the _process_scan of GridController - I suspect it could definitely be improved.