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

Update getAnnotation.R #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

garethgillard
Copy link

I was getting errors that the FTP download was not completing. The same as this issue from 2 years ago: #27. I found this was because of how the Bgee database and R package is set up now, the path that is set to where files are downloaded and extracted (pathToData), is the same path that line 76 of getAnnotation.R script deletes by unlinking (dirname(myData)). This removes all downloaded BGee data when running and therefore returns an error. I guess this line was left over from a previous set up. I think just removing will be a fix with no problems.

I was getting errors that the FTP download was not completing. The same as this issue from 2 years ago: BgeeDB#27. I found this was because of how the Bgee database and R package is set up now, the path that is set to where files are downloaded and extracted (pathToData), is the same path that line 76 of getAnnotation.R script deletes by unlinking (dirname(myData)). This removes all downloaded BGee data when running and therefore returns an error. I guess this line was left over from a previous set up. I think just removing will be a fix with no problems.
@jwollbrett
Copy link
Collaborator

Hi @garethgillard
Thanks for your feedback.
I looked at the error and managed to reproduce it when using a tilde in the declaration of the variable pathToData.
I commited a hotfix to the develop and master branches of our github repo and also to the devel and RELEASE_3_19 (current production branch) of Bioconductor.
This hotfix replaces the tilde in pathToData using the function path.expand()

@garethgillard
Copy link
Author

Hi @jwollbrett, thanks for looking into this. Good to find and one bug, unfortunately there is still the problem with the line of code I highlighted in this pull request. It deletes all downloaded data right after downloading it, which makes the package unusable. I'm making a little tutorial on using Bgee and would very much like to showcase using the R package. I recommend updating the package with the pull request to fix it.
Many thanks for the help

@jwollbrett
Copy link
Collaborator

Hi @garethgillard,

I did not manage to reproduce the bug described in #27 . I tried both using the master branch of our git repository and the last Bioconductor release (3.19).
Could you please provide the version of the BgeeDB R package that you are using and your R version?
Thank you

@garethgillard
Copy link
Author

I have some more information around the problem, different versions give different results (R version = 4.4.1).

The Master branch and Bioconductor 3.19 are version 2.30.1 of the BgeeDB package which link still to Bgee release 15_0.

To get the 15_2 release, I can use the devel version on Bioconductor:
BiocManager::install(version='devel') BiocManager::install("BgeeDB")

This is version 2.31.0 of the BgeeDB package, and it's this version that's giving me errors with geneAnnotation() or getData().

If I use the git develop branch however (remotes::install_github("https://github.com/BgeeDB/BgeeDB_R", ref = "develop")), this is version 2.31.1. I do not get any error and have release 15_2 data, so this version is all good!

@jwollbrett
Copy link
Collaborator

Hi @garethgillard,

In general I do not recommend to use the bioconductor devel branch especially to create a tutorial. It is the branch we use to test that our new implementations pass the bioconductor build so you can have issues working with it.

I am currently working on the bioconductor devel branch and did not check the build process in Bioconductor. It looks I had an error in the documentation of an example. I solved that error and the next build should solve the issue on that branch too. It can take up to 48h.

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.

2 participants