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

Read CID/EAD mode from wiff2 files #3087

Merged
merged 15 commits into from
Aug 1, 2024
Merged

Conversation

david-cox-sciex
Copy link
Contributor

Use DataAPI for wiff2 to get detailed experiment information which includes fragmentation mode for spectra.

Copy link
Member

@chambm chambm 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 except tabs need to be converted to spaces. Can this wait for the EAD energy term to be approved and the CV updated in pwiz? I will do the latter although it may be more involved than usual because it looks like they've been messing around with embedding other ontologies inside the OBO.

@david-cox-sciex
Copy link
Contributor Author

I'll take care of the tabs to spaces now. I think the EAD term is now in psi-ms.

@david-cox-sciex
Copy link
Contributor Author

@chambm
I converted tabs to spaces.

Should I run the update_cv.bat, and then include the file changes it generates? I see it bringing in the electron beam energy term, it also makes some other changes to obo file that I think are okay but I'm not certain.

@chambm
Copy link
Member

chambm commented Jul 24, 2024

I will handle that part. I'm waiting for the temperature term to go in so I can do that later without having another CV update. Since you ran the update already, does it actually compile?

@david-cox-sciex
Copy link
Contributor Author

Yes. I run this and get a functioning MSConvertGUI.exe
quickbuild.bat --i-agree-to-the-vendor-licenses address-model=64 debug

@chambm chambm self-requested a review July 26, 2024 14:48
@chambm
Copy link
Member

chambm commented Jul 26, 2024

One thing that's missing here is a new test data set. Do you think you can get a tiny WIFF/WIFF2 set with a few EAD spectra? One neat feature of Thermo QualBrowser I discovered is it can export a single spectrum as a RAW file. The spectrum metadata is preserved although file-level metadata is lost. But that's still useful for testing that different spectrum types are converted properly! I'm not sure if Sciex software has any analogous capability.

@david-cox-sciex
Copy link
Contributor Author

I have a ~15 MB file I've been using as a manual test.
Can you point me at where the tests are defined?

@chambm
Copy link
Member

chambm commented Jul 26, 2024

Reader_ABI_Test.cpp . All WIFF and WIFF2 files in the .data directory are tested. 15MB is rather big though. Any way to get it down to just a few spectra?

@david-cox-sciex
Copy link
Contributor Author

that is the smallest one I can find.

Should I put these files in
\pwiz\data\vendor_readers\ABI\Reader_ABI_Test.data

7600ZenoTOFMSMS_EAD_TestData.zip

@chambm
Copy link
Member

chambm commented Jul 26, 2024

OK, if you've reviewed the mzML and it's converting ok, go ahead and add it. Thanks!

@david-cox-sciex
Copy link
Contributor Author

OK. the example files are added. If everything is okay, will you (@chambm) be doing the merge. For me it is blocked.

@david-cox-sciex
Copy link
Contributor Author

@chambm
Copy link
Member

chambm commented Jul 30, 2024

Sorry I was out sick yesterday. Is this a DIA file? I'm wondering why it's not getting isolation width.

@david-cox-sciex
Copy link
Contributor Author

Not DIA, just a looped MSMS. It's possible we only show isolation width on DIA experiments.

@chambm
Copy link
Member

chambm commented Jul 31, 2024

It turns out that only all WIFF files in that directory are tested; WIFF2 files have to be added by name. I also added some index subsetting to keep the size of the reference mzML down.

@chambm chambm merged commit d587dc1 into ProteoWizard:master Aug 1, 2024
12 checks passed
@chambm
Copy link
Member

chambm commented Aug 5, 2024

Thanks for adding this @david-cox-sciex !

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