glasgowcompbio / vimms

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

Remove scan param creation methods from the environment #121

Closed joewandy closed 4 years ago

joewandy commented 4 years ago

Would be nice if get_default_scan_params and get_dda_scan_param were to be moved out from the environment. Currently it leads to a pretty awkward code, such as the following when making a FixedScansController:

        controller = FixedScansController(ionisation_mode, [])
        mass_spec = IndependentMassSpectrometer(ionisation_mode, two_fixed_chems, None)
        env = Environment(mass_spec, controller, min_rt, max_rt)

        ms1_scan = env.get_default_scan_params()
        ms2_scan_1 = env.get_dda_scan_param(mz_to_target[0], 0.0, None, isolation_width, mz_tol, rt_tol)
        ms2_scan_2 = env.get_dda_scan_param(mz_to_target[1], 0.0, None, isolation_width, mz_tol, rt_tol)
        ms2_scan_3 = env.get_dda_scan_param(mz_to_target, [0.0, 0.0], None, isolation_width, mz_tol, rt_tol)

        schedule = [ms1_scan, ms2_scan_1, ms2_scan_2, ms2_scan_3]
        controller.schedule = schedule

because we need to create the env first to get access to get_dda_scan_param in order to create the tasks.

The problem is inside get_default_scan_params and get_dda_scan_param, there's this dependence on polarity (also called 'ionisation mode'), which is currently stored inside the mass spec.

        default_scan_params = ScanParameters()
        ...
        default_scan_params.set(ScanParameters.POLARITY, self.mass_spec.ionisation_mode)
        return default_scan_params

So if we move this method elsewhere or make it static, we need to pass this polarity information somehow .. Need some thoughts. Also, polarity is actually specified for each custom scan, so it doesn't need to be stored globally for the entire mass spec. This prevents us from doing e.g. a pos/neg switching controller.

sdrogers commented 4 years ago

I think we just need a comment in the FixedController saying that it should not assume that the sequence will be provided in the consructor...

joewandy commented 4 years ago

Done the above as part of PR #155