noamross
commented
7 years ago Editor checks:
- [x] Fit: The package meets criteria for fit and overlap
- [x] Automated tests: Package has a testing suite and is tested via Travis-CI or another CI service.
- [x] License: The package has a CRAN or OSI accepted license
- [x] Repository: The repository link resolves correctly
- [ ] Archive (JOSS only, may be post-review): The repository DOI resolves correctly
- [ ] Version (JOSS only, may be post-review): Does the release version given match the GitHub release (v1.0.0)?
Thank you for the submission @bmschmidt! Traditionally something like wordVectors is a bit out-of-scope for us, but we are piloting accepting more methods-focused packages within some select sub-fields, starting with text mining. As such @lmullen will help out as co-editor here to find appropriate reviewers and ensure that implementation is well-reviewed and consistent with published methods.
@lmullen, can you give your thoughts on the additional utility of this package compared to the others above, as well as its relationship to other ones such as text2vec or h20?
Below is output from goodpractice::gp(). I note among other things that testing coverage is relatively low. covr::shine(covr::package_coverage() is a good way to look at what areas of code may be better tested. Reviewers may suggest additional testing. I suggest you integrate coverage testing into your CI (more info here) and add a coverage badge - we're moving to requiring this of all packages soon.
Using camelCase is fine to ensure back-compatibility. For Windows, if you're not already, consider using AppVeyor for CI or rhub for intermittent testing. The C warning issues we'll aim to address in review.
Nothing above is critical but I suggest you address these issues as you can while we finish editor's review and then seek reviewers.
── GP wordVectors ──────────────────────
It is good practice to
✖ write unit tests for all functions, and all package code in general. 14% of code lines are
covered by test cases.
R/matrixFunctions.R:41:NA
R/matrixFunctions.R:85:NA
R/matrixFunctions.R:86:NA
R/matrixFunctions.R:87:NA
R/matrixFunctions.R:88:NA
... and 915 more lines
✖ omit "Date" in DESCRIPTION. It is not required and it gets invalid quite often. A build date
will be added to the package when you perform `R CMD build` on it.
✖ add a "URL" field to DESCRIPTION. It helps users find information about your package online. If
your package does not have a homepage, add an URL to GitHub, or the CRAN package package page.
✖ add a "BugReports" field to DESCRIPTION, and point it to a bug tracker. Many online code hosting
services provide bug trackers for free, https://github.com, https://gitlab.com, etc.
✖ use '<-' for assignment instead of '='. '<-' is the standard, and R users and developers are
used it and it is easier to read your code for them if you use '<-'.
R/matrixFunctions.R:9:14
R/matrixFunctions.R:35:17
R/matrixFunctions.R:77:19
R/matrixFunctions.R:81:34
R/matrixFunctions.R:108:10
... and 102 more lines
✖ avoid long code lines, it is bad for readability. Also, many people prefer editor windows that
are about 80 characters wide. Try make your lines shorter than 80 characters
R/matrixFunctions.R:85:1
R/matrixFunctions.R:137:1
R/matrixFunctions.R:190:1
R/matrixFunctions.R:220:1
R/matrixFunctions.R:275:1
... and 23 more lines
✖ avoid the library() and require() functions, they change the global search path. If you need to
use other packages, import them. If you need to load them explicitly, then consider
loadNamespace() instead, or as a last resort, declare them as 'Depends' dependencies.
R/word2vec.R:143:7
✖ fix this R CMD check NOTE: 'library' or 'require' call to ‘stringi’ in package code. Please use
:: or requireNamespace() instead. See section 'Suggested packages' in the 'Writing R Extensions'
manual. Namespace in Imports field not imported from: ‘graphics’ All declared Imports should be
used.
✖ fix this R CMD check NOTE: project: no visible global function definition for ‘new’
square_magnitudes: no visible global function definition for ‘.hasSlot’
-,VectorSpaceModel-VectorSpaceModel: no visible global function definition for ‘callNextMethod’
-,VectorSpaceModel-VectorSpaceModel: no visible global function definition for ‘new’
[,VectorSpaceModel: no visible global function definition for ‘callNextMethod’
[,VectorSpaceModel: no visible global function definition for ‘new’ initialize,VectorSpaceModel:
no visible global function definition for ‘callNextMethod’ plot,VectorSpaceModel: no visible
global function definition for ‘predict’ plot,VectorSpaceModel: no visible global function
definition for ‘prcomp’ plot,VectorSpaceModel: no visible global function definition for ‘text’
Undefined global functions or variables: .hasSlot callNextMethod new prcomp predict text
Consider adding importFrom("graphics", "text") importFrom("methods", ".hasSlot",
"callNextMethod", "new") importFrom("stats", "prcomp", "predict") to your NAMESPACE file (and
ensure that your DESCRIPTION Imports field contains 'methods').
✖ fix this R CMD check WARNING: Note: significantly better compression could be obtained by using
R CMD build --resave-data old_size new_size compress demo_vectors.rda 927Kb 643Kb bzip2
✖ fix this R CMD check NOTE: Found the following hidden files and directories: .travis.yml These
were most likely included in error. See section ‘Package structure’ in the ‘Writing R
Extensions’ manual. (use .Rbuildignore for this)
✖ avoid 'T' and 'F', as they are just variables which are set to the logicals 'TRUE' and 'FALSE'
by default, but are not reserved words and hence can be overwritten by the user. Hence, one
should always use 'TRUE' and 'FALSE' for the logicals.
R/matrixFunctions.R:NA:NA
R/matrixFunctions.R:NA:NA
R/word2vec.R:NA:NA
R/word2vec.R:NA:NA
R/word2vec.R:NA:NA
... and 7 more lines
───────────────────(Also devtools::spell_check() found "interepreted" at nearest_to.Rd:38)
Reviewers: @juliasilge @jeroenooms @tedunderwood Due date: 2017-03-07
jeroen
bmschmidt
juliasilge
tedunderwood
sckott
Summary
What does this package do? (explain in 50 words or less):
This package wraps Google's implementation in C for training word2vec models, and provides several R functions for exploratory data analysis of word2vec and other related models. These include import-export from binary, basic linear algebra missing from R, and a streamlined syntax for working with models and performing vector arithmetic.
Paste the full DESCRIPTION file inside a code block below:
https://github.com/bmschmidt/wordVectors/tree/dev
(Note that this is a dev branch of the repo I'm submitting: I've made a couple breaking changes to the API for review, but don't want to inconvenience current users if more are in the pipeline.)
People working with large textual collections who want to explore them with word2vec models they train themselves.
People doing NLP tasks who want to incorporate existing models (like Google's news vectors) into their pipeline.
Workshops, classrooms, and other situations where people want to promote exploration of word2vec or other models.
I built off of an existing package here on which development seems to be abandoned. My package has fewer bugs, and offers access to more of the underlying bindings. There's another package offering access to all the word2vec bindings here; it's maintainer has contributed patches to this project, and I think their functionality is roughly comparable on the core functions.
The big difference is that unlike either of these, my package offers import/export in the binary format, and wrappers for cosine similarity and the like, with syntactic sugar for word2vec models.
Requirements
Confirm each of the following by checking the box. This package:
Publication options
paper.mdwith a high-level description in the package root or ininst/.(how do I get a long-term repository for JOSS?)
Detail
R CMD check(ordevtools::check()) succeed? Paste and describe any errors or warnings:Two warnings:
Number one has only recently appeared: the instructions (
R CMD build --resave-data) seem not to work.More seriously (?), some of the word2vec C code throws warnings because they use read/write functions. I don't know R's C bindings well enough to fix these; many people are currently using it in production.
Build is tested on Ubuntu and OS X. I've also heard reports of builds failing on certain Windows versions. There's been much less of this since a few recent patches and I believe it to be fixed, but I don't have access to a test suite across old Windows versions to be sure.
If this is a resubmission following rejection, please explain the change in circumstances:
If possible, please provide recommendations of reviewers - those with experience with similar packages and/or likely users of your package - and their GitHub user names:
@lmullen has used the package and has contributed to you all a lot. He could perhaps suggest other uses.