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

Add the option to remove retention indices before initializing them from the comment #120

Closed
2 tasks done
hechth opened this issue Apr 18, 2024 · 0 comments · Fixed by #122
Closed
2 tasks done

Add the option to remove retention indices before initializing them from the comment #120

hechth opened this issue Apr 18, 2024 · 0 comments · Fixed by #122
Assignees

Comments

@hechth
Copy link
Member

hechth commented Apr 18, 2024

This function could be implemented in multiple ways:

  • in init_ri_from_comment to check whether the keyword for RI type to assign is present or not, and if it is not presend and the RI value is set, then remove it.
  • allow cleaning the RI upon reading the data in _read in MatchMSData and PandasData
  • by adding a clean_ri function which sets all RI values in a Data object to 0 or None
  • not implement anything and just use the setter and set all RI values to None by passing an array of Nones to the setter

Problem is, that RI values which don't represent actual values are not written by the package, so it is actually not possible to remove RI values with the package. This does not concern the pandas data.
The current implementation of the write function of the MatchMSData class doesn't support writing the value if it is smaller 0.

If we want to allow resetting values etc., we should remove the RI key if the value is 0. The RI values for spectra where there is non numeric information stored is currently set to 0. So this means removing the key from the spectrum in the _assign_ri_values function.

TLDR

  • add the else statement to remove the reteion index key if the value is 0
  • implement unit test for this behaviour and make sure that all other tests still pass
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 a pull request may close this issue.

2 participants