sba1 / mgsa-bioc

Mirror of the mgsa Bioconductor package with full history in the master branch. Content of https://hedgehog.fhcrc.org/bioconductor/trunk/madman/Rpacks/mgsa in the bioconductor branch.
5 stars 5 forks source link

mgsa method not passing on the 'p' argument correctly ? #4

Closed tomsing1 closed 9 years ago

tomsing1 commented 9 years ago

I noticed a change in behavior of the 'mgsa' method in your 'mgsa' package between the current release and development versions.

In the release version of R (see sessionInfo below), the following command works:

mgsa(
   o=c("A", "B"), 
   sets=list(set1 = LETTERS[1:3], set2 = LETTERS[2:4]), 
   p=0.1
)

In the devel version (again, see sessionInfo below), the same command throws an exception, which refers to the 'population' argument.

mgsa(
   o=c("A", "B"), 
   sets=list(set1 = LETTERS[1:3], set2 = LETTERS[2:4]), 
   p=0.1
)
Error in mgsa.main.debug(o, sets, population, ...) :
  'population' must be NULL or have the same class as 'o'.

When I checked the changes between the release and devel versions of the mgsa:::mgsa.main function, I noticed that the former lists the available parameters explicitly, while the latter uses the '...' notations.

I suspect that some partial matching within the parameters provided as '...' may confuse the 'p' and 'population' arguments. Would you mind taking a look ?

Thanks a lot ! Thomas

---- SessionInfo for release version

R version 3.1.1 (2014-07-10)
Platform: x86_64-unknown-linux-gnu (64-bit)

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C
 [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C
 [9] LC_ADDRESS=C               LC_TELEPHONE=C
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

other attached packages:
[1] mgsa_1.14.2          gplots_2.14.2        BiocInstaller_1.16.1
[4] roxygen2_4.0.2       devtools_1.6.1

loaded via a namespace (and not attached):
[1] bitops_1.0-6       caTools_1.17.1     compiler_3.1.1     gdata_2.13.3
[5] gtools_3.4.1       KernSmooth_2.23-13 Rcpp_0.11.3        stringr_0.6.2
[9] tools_3.1.1
##---- SessionInfo for devel version
R Under development (unstable) (2014-11-01 r66923)
Platform: x86_64-unknown-linux-gnu (64-bit)

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C
 [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C
 [9] LC_ADDRESS=C               LC_TELEPHONE=C
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

other attached packages:
[1] mgsa_1.15.2          gplots_2.16.0        BiocInstaller_1.17.3
[4] roxygen2_4.1.0       devtools_1.6.1

loaded via a namespace (and not attached):
[1] bitops_1.0-6       caTools_1.17.1     compiler_3.2.0     gdata_2.13.3
[5] gtools_3.4.1       KernSmooth_2.23-13 Rcpp_0.11.3        stringr_0.6.2
[9] tools_3.2.0
sba1 commented 9 years ago

Seems that 'p' is interpreted as a short cut for 'population'. In the devel version, the 'p' argument isn't listed explictly anymore. Instead, the three-dots is used.

It should work if you supply population=NULL, but I will most likely revert the changes that led to the introduction of the three-dots.

sba1 commented 9 years ago

At http://cran.r-project.org/doc/manuals/r-release/R-lang.html#Argument-matching the rules are described.

sba1 commented 9 years ago

The bug may be fixed. It will usually take some time until the fixed verison is deployed, but you could check it by compiling the devel version yourself. Please report back if it works or not.

tomsing1 commented 9 years ago

Yes, things work again as before. Thank you for looking into this so quickly !

sba1 commented 9 years ago

Thanks for reporting the issue and confirming the fix.