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

Stian Soiland-Reyes' review for associated paper #57

Closed
peterjc opened this issue Jan 19, 2015 · 6 comments
Closed

Stian Soiland-Reyes' review for associated paper #57

peterjc opened this issue Jan 19, 2015 · 6 comments

Comments

@peterjc
Copy link
Owner

peterjc commented Jan 19, 2015

General tracking issue for code issues raised by Stian Soiland-Reyes (@stain on GitHub, soilandreyes on Twitter) while reviewing our paper NCBI BLAST+ integrated into Galaxy as submitted to GigaScience:

Stian Soiland-Reyes (Twitter: soilandreyes):
Submitted my #OpenReview of @pjacock's @galaxyproject BLAST+ paper for
@GigaScience. My apologies for being picky..
https://gist.github.com/stain/a107379fc1adbb3e392b

I'll file separate issues for changes to the paper itself - this is purely for the code side of things.

Update - preprint of paper as submitted to GigaScience is now on bioRxiv http://dx.doi.org/10.1101/014043

@peterjc
Copy link
Owner Author

peterjc commented Jan 19, 2015

One major issue raised was to do with installing NCBI BLAST databases. This would fall under the scope of #22, even if only to add a Galaxy Data Manager for fetching and installing the latest NCBI databases.

@peterjc
Copy link
Owner Author

peterjc commented Jan 19, 2015

I've filed a bug against Galaxy for the poor handling of the Tool Shed being offline which Stian hit while trying to use @bgruening's Galaxy BLAST+ Docker image: https://registry.hub.docker.com/u/bgruening/galaxy-blast/

See: https://trello.com/c/f0MWXBlM/2348-browse-valid-sheds-fails-if-toolshed-offline

peterjc added a commit that referenced this issue Jan 19, 2015
In response to reviewer's comments, see GitHub issue #57
peterjc added a commit that referenced this issue Jan 19, 2015
e.g. Make it more explicit that each tool has a README file.

We were also lacking any instructions on setting up the BLAST
databases in the README files (it is hidden away inside the sample
location files)...

Based on feedback from Stian Soiland-Reyes (see GitHub issue #57),
the main README failed to guide readers to how to install each tool.
@peterjc
Copy link
Owner Author

peterjc commented Jan 19, 2015

Stian flagged the (harmless but scary) SQLite error from run_tests.sh with SQLite as seen on TravisCI,

I have therefore not been able to verify that the supporting data
actually supports the article, beyond inspecting the Travis-CI
build logs at https://travis-ci.org/peterjc/galaxy_blast/builds ..
which except for a single error seem to be verifying the tools.
https://travis-ci.org/peterjc/galaxy_blast/builds/45137901

OperationalError: (OperationalError) unable to open database file None None

I've filed a Galaxy issue on that: https://trello.com/c/7vLsIx8q/2349-scary-run-tests-sh-error-with-sqlite

@jmchilton
Copy link
Contributor

Also galaxyproject/planemo#18. It is totally harmless but it does keep getting brought up so it needs to be addressed.

@peterjc
Copy link
Owner Author

peterjc commented Jan 20, 2015

Stian asked Which version of the BLAST tool was used?, which Galaxy already records via the <version_command> tag (available to view via the information icon for any dataset in your current history).

However, Galaxy doesn't currently check that matches the version the wrapper was expecting - which I am suggesting with https://trello.com/c/9nz482vx/2352-verify-tool-versions-match-what-was-expected

@peterjc
Copy link
Owner Author

peterjc commented Nov 5, 2015

The revised paper is out, and we've filed issues for the outstanding issues, so I'm going to close this now.

@peterjc peterjc closed this as completed Nov 5, 2015
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

No branches or pull requests

2 participants