wkumler / RaMS

R-based access to Mass-Spectrometry data
Other
22 stars 7 forks source link

add support for reading thermo dad data #11

Closed ethanbass closed 1 year ago

ethanbass commented 2 years ago

Incorporates support to parse thermo DAD data. Closes wkumler/RaMS#10

codecov-commenter commented 2 years ago

Codecov Report

Base: 85.41% // Head: 84.11% // Decreases project coverage by -1.29% :warning:

Coverage data is based on head (1caf9e2) compared to base (d4c4d98). Patch coverage: 44.11% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #11 +/- ## ========================================== - Coverage 85.41% 84.11% -1.30% ========================================== Files 5 5 Lines 1083 1114 +31 ========================================== + Hits 925 937 +12 - Misses 158 177 +19 ``` | [Impacted Files](https://codecov.io/gh/wkumler/RaMS/pull/11?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=William) | Coverage Δ | | |---|---|---| | [R/grabMzmlFunctions.R](https://codecov.io/gh/wkumler/RaMS/pull/11/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=William#diff-Ui9ncmFiTXptbEZ1bmN0aW9ucy5S) | `81.40% <40.00%> (-4.93%)` | :arrow_down: | | [R/grabMSdataCode.R](https://codecov.io/gh/wkumler/RaMS/pull/11/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=William#diff-Ui9ncmFiTVNkYXRhQ29kZS5S) | `87.70% <75.00%> (-0.43%)` | :arrow_down: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=William). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=William)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

ethanbass commented 2 years ago

let me know if you want me to try to write some tests or something. not sure how that would work without a file? the files are huge!

wkumler commented 2 years ago

This is neat! The code looks good and I like your implementation of it as another thing that can be requested using grab_what. I'll see what I can do to hack together a file for unit tests because I think this is important data to access and having unit tests will ensure that the functionality doesn't get broken further down the line. Feels similar to the way we're implementing chromatograms in #8. Might be a little while since I'm working on some fieldwork now but it's now on my to-do list. Thanks for writing the code!

ethanbass commented 2 years ago

sounds good to me. glad you like the code!

wkumler commented 1 year ago

Ok, I've made a "minified" DAD file from one of the files you sent me consisting of the first 5 MS scans and the first 5 DAD scans - that should be enough to run unit tests and ensure later contributions don't break DAD processing. It's also nice that you have both pos and neg mode in those files, that's something I hadn't thought to test for before. Are you ok with this minified file being included in the package? 20220404_CirA_D5_30_mini.mzML.gz

ethanbass commented 1 year ago

Awesome thanks for doing that! I think it's probably fine. I actually got the file from Ben Reisman, so I just sent him a message to confirm. Will let you know when i hear back

On Mon, Nov 7, 2022 at 6:01 PM William @.***> wrote:

Ok, I've made a "minified" DAD file from one of the files you sent me consisting of the first 5 MS scans and the first 5 DAD scans - that should be enough to run unit tests and ensure later contributions don't break DAD processing. It's also nice that you have both pos and neg mode in those files, that's something I hadn't thought to test for before. Are you ok with this minified file being included in the package? 20220404_CirA_D5_30_mini.mzML.gz https://github.com/wkumler/RaMS/files/9956193/20220404_CirA_D5_30_mini.mzML.gz

— Reply to this email directly, view it on GitHub https://github.com/wkumler/RaMS/pull/11#issuecomment-1306339835, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADZEBOY6ATU5RDZODFSRKELWHGC5LANCNFSM6AAAAAAQPGSQ2Q . You are receiving this because you authored the thread.Message ID: @.***>

ethanbass commented 1 year ago

He said it's fine to use the file

wkumler commented 1 year ago

Nice! It's now live in the latest version of the package.