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 ion mobility attribute as parent for CCS #230

Merged
merged 4 commits into from
Dec 19, 2023
Merged

Add ion mobility attribute as parent for CCS #230

merged 4 commits into from
Dec 19, 2023

Conversation

chambm
Copy link
Contributor

@chambm chambm commented Dec 15, 2023

As discussed on PSI call, we want to allow CCS to be encoded in mzML wherever individual ion mobility values are currently allowed.

@chambm chambm requested a review from edeutsch December 15, 2023 19:14
@edeutsch edeutsch requested a review from mobiusklein December 16, 2023 00:13
@edeutsch
Copy link
Contributor

This looks good, thanks. It was mentioned at the call today that several vendor APIs allow for computing the CCS when reading vendor datafiles, so this could be easily encoded in the mzMLs.

Copy link
Contributor

@mobiusklein mobiusklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. There's a degree of approximation error that we don't really understand but there's nothing to be done about it. Because CCS requires knowledge of charge state, it doesn't yet seem feasible to encode on a peak level.

@edeutsch
Copy link
Contributor

but this is for precursors I think, right? where the charge state is ostensibly known?

@chambm
Copy link
Contributor Author

chambm commented Dec 19, 2023

Yes, it only makes sense when charge state is relevant. So unlike drift time et al., it actually doesn't make sense in <scan>, only in <selectedIon>. However, it is feasible to have an array of CCS values just like we can have an array of charge states. Unknown values could be stored as 0. I think we can leave that for another PR though.

Because CCS only makes sense in combination with a charge state.
@chambm chambm added this pull request to the merge queue Dec 19, 2023
Merged via the queue into master with commit 93afe9c Dec 19, 2023
2 checks passed
@chambm chambm deleted the chambm-patch-1 branch December 19, 2023 16:23
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.

3 participants