glasgowcompbio / vimms

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

Noise class is a bit restrictive? #69

Closed sdrogers closed 4 years ago

sdrogers commented 4 years ago

Current Noise class is a single object passed to the MS.

I can easily imagine that we will want different noise for MS1, and MS2, and for mz and intensity.

Suggest a bit of a refactoring so that the MS takes two objects (one to generate noise for mz and one for intensity). In practice, the same object could be passed to both -- the noise generator doesn't care if it is making noise for m/z or intensity.

Each noise generator is passed the ms_level and can decide if it wants to behave differently for different levels.

Any thoughts?

sdrogers commented 4 years ago

as an aside -- doc string in MS is now out of date

sdrogers commented 4 years ago

Also, is this code now redundant? https://github.com/sdrogers/vimms/blob/951fc55311f3a6409c3397b6a2b8cd6fa7f3081e/vimms/MassSpec.py#L577-L587

joewandy commented 4 years ago

Suggest a bit of a refactoring so that the MS takes two objects (one to generate noise for mz and one for intensity). In practice, the same object could be passed to both -- the noise generator doesn't care if it is making noise for m/z or intensity.

Sounds cleaner. Let's go with this.

joewandy commented 4 years ago

Also, is this code now redundant? https://github.com/sdrogers/vimms/blob/951fc55311f3a6409c3397b6a2b8cd6fa7f3081e/vimms/MassSpec.py#L577-L587

Yeah it's no longer used anywhere, and can be deleted.