NESCent / popgenInfo

Vignettes for Population Genetics in R
http://popgen.nescent.org
MIT License
20 stars 50 forks source link

genind2hierfstat is no longer exported and no longer needed #78

Closed hlapp closed 8 years ago

hlapp commented 8 years ago

The function genind2hierfstat has been removed from adegenet, as hierfstat now deals natively with genind objects: http://lists.r-forge.r-project.org/pipermail/adegenet-forum/2015-September/001285.html

This results in the following type of errors:

Quitting from lines 80-87 (DifferentiationSNP.Rmd) 
Error in eval(expr, envir, enclos) : 
  could not find function "genind2hierfstat"
smanel commented 8 years ago

I try to use genind objects directly with hierfstat in the vignette. It does not work in the case of the commands used in the vignette. I am waiting an answer from J. Goudet.

bjcochrane commented 8 years ago

genind2hierfstat is indeed gone from adegenet, however hierfstat version 0.4-14, the most recent on CRAN, does not read genind objects (at least for the basic.stats function). Is there a developer version somewhere that does?

zkamvar commented 8 years ago

Hi @bjcochrane, @hlapp, and @smanel

Sorry for the late reply, I've been buried in work and then a much needed holiday. genind2hierfstat() now exists on the development version of hierfstat, which you can install using devtools:

devtools::install_github("jgx65/hierfstat")

hope that helps.

hlapp commented 8 years ago

So @zkamvar, was the resolution that genind2hierfstat() does need to be exported from hierfstat? I.e., it did exist in the package since July, it just wasn't exported, with the reasoning that it shouldn't be needed because users shouldn't have to worry about interoperability. I think the functions that are used in the vignette accept genind objects and convert automatically (as they should) - was that deemed not desirable to implement for those functions that apparently haven't done so yet?

I'd like to avoid having to mix CRAN and development packages in the container that renders the website (and that thus runs the tests). That's because most users who would be a target audience for the vignettes aren't likely to mix at will either. But we can of course discuss.

zkamvar commented 8 years ago

@hlapp, I believe the original idea was to port genind2hierfstat() to hierfstat so that genind objects would be passed to genind2hierfstat() internally. It was placed in hierfstat on June 22nd as an internal function called .genind2hierfstat(). However, hierfstat has not been updated on CRAN in over a year and it only recently gained the ability to work with genind objects for many of the functions since this commit six days ago.

Additionally, the author has stated that hierfstat will be on CRAN by the end of the year and has asked for people to check their data with the functions.

I agree that we want to avoid mixing CRAN and github packages, but until these are submitted to CRAN, there is very little we can do. Is it possible to have a development version of the website?

jgx65 commented 8 years ago

Hi all, I'm currently writing a short vignette explaining the latest developments in hierfstat. As soon as this is done, I'll put it on CRAN. Hope this is ok for you?

jgx65 commented 8 years ago

Currently, we cannot avoid completely using hierfstat::genind2hierfstat because some functions (e.g. test.g, varcomp.glob ) require two data frames, one for loci and one for populations or levels. Several others (basic.stats, wc, genet.dist, ind.pca,...) work with genind objects.

hlapp commented 8 years ago

I updated the Docker file underlying the image that Circle CI uses, and the Docker image is now rebuilt.

When rebuilding with that new image, the original error disappears, but a new one appears. @smanel had already committed the fix to DifferentiationSNP.md, which I subsequently moved from master to a branch, from which I now created a pull request (#79) so we can track progress and build status there. Once we get that to a passing state, we can merge into master, which will hopefully make that build pass again, too, and I'll then pull the changes into the microsatellites branch on which @smanel has been working on a vignette.

jgx65 commented 8 years ago

@hlapp @smanel @zkamvar Latest version of hierfstat (0.04.20) submitted to CRAN.

zkamvar commented 8 years ago

:+1: :confetti_ball: :tada: :100:

jgx65 commented 8 years ago

It's now that I'm gonna get flamed ;-)

hlapp commented 8 years ago

This has been fixed through #79 and #87.