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

xcms3 as a branch or a completely rewritten package? #29

Closed
jorainer opened this issue Jul 18, 2016 · 9 comments
Closed

xcms3 as a branch or a completely rewritten package? #29

jorainer opened this issue Jul 18, 2016 · 9 comments
Labels

Comments

@jorainer
Copy link
Collaborator

@lgatto @sneumann
So far discussions about that have been mostly email-based; time to have them in public.
The main question is whether it might be better to a) develop xcms3 as a branch of xcms replacing it in the long run or b) develop xcms3 as a completely new package.

Pros for a):

  • keep the large user-community of xcms.

Cons for a):

  • need to keep old functionality, then deprecate etc.

Pros for b):

  • xcms would be untouched, users can still use it without any deprecation warnings etc. Both packages could live in parallel.

Cons for b):

  • New users might choose xcms3 over xcms; old users might stick with xcms.
@jorainer jorainer added the xcms3 label Jul 18, 2016
@stanstrup
Copy link
Contributor

I guess the answer really depends on how radical the change. The more radical the more b makes sense.
MzMine is going through a similar process of re-writing everything and as far as I understand they went for b and is slowly porting over features.

@lgatto
Copy link
Contributor

lgatto commented Jul 18, 2016

A in between alternative would be the following:

Start new development in a xcmsdev package/repository to have the clean sheet advantage. Once that package has reached some basic functionality, xcms would depend on/import it and it would get the necessary wrappers/infrastructure to use it. (I believe it is possible to make use of github packages, although I haven't done it yet). Depending on maturity/stability, the xcmsdev code could even be moved to xcms and old xcms code deprecated/removed. At some point, dependency on xcmsdev could be removed too.

Although xcmsdev could be functional on its own (albeit possibly concentrating on low-level functionality), a typical user would be expected to use xcms rather than the xcmsdev.

@jorainer
Copy link
Collaborator Author

@stanstrup : the changes I would like to introduce are:

  • Use MSnbase's OnDiskMSnExp object to read and represent the raw data (without actually keeping it in memory); this should be similar to the xcmsRaw object, but without all the data in memory.
  • Methods (e.g. detectFeatures) that take the raw data object from above to identify the features. Dispatching to the various algorithms (e.g. centwave) would be done using a parameter class (e.g. CentWaveParam).

This will already change the xcms workflow considerably; still, I would be fine with each option.

@jorainer
Copy link
Collaborator Author

@lgatto : I like that idea, however, I don't know how github dependencies are seen in Bioconductor... based on the latest discussions at BioC2016 not so well...

@sneumann
Copy link
Owner

Hi, I am leaning to doing the changes in-place. That's what development cycles are for.
W.r.t. the timing, disruptive changes and/or those that might change the outcomes
(peak picking, alignment etc.) should be early in the BioC development cycles.

@jorainer
Copy link
Collaborator Author

I can live will all options; as detailed in #30 I am planning to separate the UI from the actual analysis functions hence making the code as modular as possible.
I am only afraid that users might get upset if some of the methods are removed at some point.

@lgatto
Copy link
Contributor

lgatto commented Jul 18, 2016

I don't know how github dependencies are seen in Bioconductor... based on the latest discussions at BioC2016 not so well...

I am aware of the Remotes entry in the DESCRIPTION file: see https://github.com/hadley/devtools/blob/master/vignettes/dependencies.Rmd. Don't know how/if this work with biocLite though.

@jorainer
Copy link
Collaborator Author

OK, so, following @sneumann's advice I'll start doing under the hood changes within xcms first (i.e. introducing the layer of do_ functions as detailed in #30).
Also, I would like to do some clean up and re-organization of the existing code:

  • Fix the warnings of R CMD check
  • Re-organize the R files (i.e. Classes.R, methods-...R, functions-...R)

Any objections?

@jorainer
Copy link
Collaborator Author

Closing this as we're already on our way towards xcms3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants