Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix min and max mz filtering in cut_mz_domain_noise #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Kzra
Copy link
Contributor

@Kzra Kzra commented Mar 6, 2023

I noticed that changing the min_mz_noise and max_mz_noise parameters did not effect the baseline noise calculation when the threshold method was 'signal_noise'.

This seems to be fixed by changing the indexing in the cut_mz_domain_noise method in the NoiseThresholdCalc class.
Because mz_exp_profile was ordered from high to low in my tests, the indexing in the following commands needed to be reversed.

So I changed

 low_mz_index = (where(self.mz_exp_profile >= min_mz_noise)[0][0])
 high_mz_index = (where(self.mz_exp_profile <= max_mz_noise)[-1][-1])

to:

low_mz_index = (where(self.mz_exp_profile >= min_mz_noise)[-1][-1])
high_mz_index = (where(self.mz_exp_profile <= max_mz_noise)[0][0]) 

I saw that there was an if statement at the end of the function which compares the low_mz_index and high_mz_index to determine the order of the slice. This implies that the order of mz_exp_profile is not always fixed, in which case a more flexible solution is required. Happy to implement this if this is the case, otherwise I think the current fix will be fine.

I also changed the if statement at the end of the function which had the low_mz_index and high_mz_index variables in the wrong order.

EDIT 07/03/23: Changed the first index to -1 for low_mz_index in case the mz_exp_profile is a 2D array

@wkew
Copy link

wkew commented Oct 24, 2023

mz should be ordered from low to high mz by default now, so this may not be required anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants