search

nsh87 / receptormarker

Source for 'receptormarker' package for R: antibody receptor and phenotypic marker analysis
http://receptormarker.com
BSD 2-Clause "Simplified" License
4 stars 9 forks source link

[Opened as New PR] Change determination of optimal clusters #28

Closed catterbu closed 9 years ago

catterbu commented 9 years ago

By leveraging the NbClust package, determination of the optimal number of clusters is done by looking at 26 different metrics and choosing the most common one. Note: When running devtools::check(), there was a warning regarding the NbClust package. See below. Unknown package 'DbClust' in Rd xrefs

nsh87 commented 9 years ago

Add the latest version of DbClust to Imports, should fix it. Looks like there's a cross reference, I imagine a link, in the NbClust documentation to it.

nsh87 commented 9 years ago

If that works the Import should really be in NbClust Description.

catterbu commented 9 years ago

DbClust does not exist. Are you sure that it was not another name? I could not find it on CRAN and I tried it without a version number and devtools::check() told me that it was not available.

nsh87 commented 9 years ago

Then you added a reference to DbClust in your documentation :wink:

nsh87 commented 9 years ago

The culprit: https://github.com/catterbu/receptormarker/blob/e2fb20978a09d7917e6082bc4d5ec2fd1f824460/man/multi_clust.Rd#L49

catterbu commented 9 years ago

I swore loudly in my head when I read this. I do not know how you found this, but thanks.

nsh87 commented 9 years ago

It's all about deciphering the error message. Can you put PCA in a different PR/branch than this one?

catterbu commented 9 years ago

Oh I forgot! How can I do that now?

nsh87 commented 9 years ago

I can't merge this until the ability to turn off NbClust's plotting is available. The NbClust plotting totally screws up the resulting plots from your wss_plot and other functions.

screen shot 2015-07-20 at 5 31 15 pm

nsh87 commented 9 years ago

I noticed that the package author isn't the same guy as the one where your PR is (https://github.com/cran/NbClust/pull/1) (actually, it's CRAN's repo, so that makes sense). That's unfortunate. I left a message on the actual author's website (https://sites.google.com/site/malikacharrad/research/nbclust-package) with a link to your PR requesting that he incorporate the changes. I'll email him and see what he says.

The other option: I checked out the license for NbClust. It's distributed under GPL-2 license, which is pretty open. So you could create your own function in our package - derived from NbClust but with your changes - and as long as

  1. You indicate the changes you make
  2. Include a copy of NbClust's GPL license in our codebase, potentially just in NbClust.R

That's an option, but maybe wait a day or two, I'm going to email the package author right now to see if he'll make the changes.

nsh87 commented 9 years ago

The NbClust guy hasn't responded... I know you had this thought already: might as well put your modifications of the code in our package. Good thing it's all open source. Just make sure to include as copy of the license file and your document your major changes in your NbClust.R, as per the GPL-2 license.

catterbu commented 9 years ago

@nsh87 if this branch passes, then it and the other two pull requests that I have open should be ready.