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

feat(metamorpheus): Create converter from Metamorpheus to core MSstats #85

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

tonywu1999
Copy link
Contributor

@tonywu1999 tonywu1999 commented Jan 18, 2024

Motivation and Context

As a part of this issue, users are requesting a converter from Metamorpheus to MSstatsPTM. The first step is to create a converter from Metamorpheus to core MSstats.

I'm placing a new converter in MSstatsConvert instead of MSstats because long term, converters should be in MSstatsConvert for the following reasons:

  • Scalability: Every time a new upstream tool is introduced, multiple packages need to be modified to create a new converter. This setup is confusing for those who do not maintain the suite of MSstats packages but want to contribute. As a result, rather than the open source community creating converters right now, we need maintainers to create the converters.
  • Timing Releases: Since multiple packages need to be modified, we must release packages to bioconductor in a particular order. We need to keep track of order of releases of multiple packages (i.e. MSstatsConvert first, MSstats next, etc.), otherwise new features could be broken without us knowing. While it's easy to keep track right now, it's not as easy to keep track if MSstats expands to the point where multiple features are being built at the same time.

Changes

  • Created a new converter called MetamorpheusToMSstatsFormat in MSstatsConvert.
  • Added a .sharedParametersAmongConverters function to store shared parameters amongst all converters

Testing

  • Added unit test to test basic functionality of MetamorpheusToMSstatsFormat

Checklist Before Requesting a Review

  • I have read this repository's contributing guidelines
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

@mstaniak
Copy link
Contributor

This function should be in MSstats package (defined there in converters.R file).

@mstaniak
Copy link
Contributor

MSstatsConvert contains MSstatsClean method for relevant file type, MSstats uses that + general functions like MSstatsPreprocess to define converters.

@mstaniak
Copy link
Contributor

Let me know if you need a clarification on this convention

@tonywu1999 tonywu1999 closed this Jan 29, 2024
@tonywu1999 tonywu1999 deleted the feature-metamorpheus-converter branch January 29, 2024 21:20
@tonywu1999 tonywu1999 restored the feature-metamorpheus-converter branch January 30, 2024 22:03
@tonywu1999 tonywu1999 reopened this Jan 30, 2024
@mstaniak
Copy link
Contributor

mstaniak commented Feb 7, 2024

A function like .sharedParametersAmongConverters exists in MSstats currently, let's keep this in mind while moving converter later.

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.

None yet

3 participants