Joss-reviews: [REVIEW]: nails: Network Analysis Interface for Literature Studies

[whedon]

Submitting author: @jsalminen (Juho Salminen)
Repository: https://github.com/aknutas/nails-package
Version: 1.0.2
Editor: @karthik
Reviewer: @jankatins
Archive: Pending

Status

Status badge code:

HTML: <a href="http://joss.theoj.org/papers/b6db586688387d912e8c6c9a86ee4894"><img src="http://joss.theoj.org/papers/b6db586688387d912e8c6c9a86ee4894/status.svg"></a>
Markdown: [![status](http://joss.theoj.org/papers/b6db586688387d912e8c6c9a86ee4894/status.svg)](http://joss.theoj.org/papers/b6db586688387d912e8c6c9a86ee4894)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer questions

Conflict of interest

• [x] As the reviewer I confirm that there are no conflicts of interest for me to review this work (such as being a major contributor to the software).

General checks

• [x] Repository: Is the source code for this software available at the repository url?
• [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• [x] Version: Does the release version given match the GitHub release (1.0.2)?
• [x] Authorship: Has the submitting author (@jsalminen) made major contributions to the software?

Functionality

• [x] Installation: Does installation proceed as outlined in the documentation?
• [x] Functionality: Have the functional claims of the software been confirmed?
• [x] Performance: Have any performance claims of the software been confirmed?

Documentation

• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g. API method documentation)?
• [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
• [x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

• [x] Authors: Does the paper.md file include a list of authors with their affiliations?
• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [x] References: Do all archival references that should have a DOI list one (e.g. papers, datasets, software)?

[whedon]

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @janschulz it looks like you're currently assigned as the reviewer for this paper :tada:.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As as reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all JOSS reviews 😿

To fix this do the following two things:

1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

For a list of things I can do to help you, just type:

@whedon commands

[aknutas]

Hello everyone. Test data for the review process is available at https://akpstorage.s3.amazonaws.com/nails_joss_testinput.zip in case any participant to the process does not have access to the web of science.

[jankatins]

Version: Does the release version given match the GitHub release (1.0.0)?

This is still a rc, so it current does not match -> needs to be addressed...

Installation and Usage

That was a bit of pain: https://github.com/aknutas/nails/issues/12

Neverthless, it worked, somehow: :white_check_mark:

Automatic tests

These are missing as far as I can see.. :question:

All in all this is a great "Product" which works great as a webservice (via http://hammer.nailsproject.net/), but the way the repo is structured for users it is a bit unusual and IMO would work much better as a package which could be installed (pulling in all required dependencies) and then used as a function.

statement of need

IMO a more "user centric" verison would be nice as the current one "undersells" the software a bit: "NAILS performs statistical and Social Network Analysis (SNA) on citation data. SNA is a new way for researchers to map large datasets and get insights from new angles by analyzing connections between articles." is correct, but IMO this is alse a great tool to simply make sense of a topic which is new to me and I want to gain a first insight what is important here and how important topics/authors are connected. Neverthelss: :white_check_mark:

citations

IMO the packages it /directly/ uses should be cited (e.g. igraph). See here how to get all citation info: http://www.astrostatistics.psu.edu/su07/R/html/utils/html/citation.html or this here has a way to get all citations of your currently loaded packaes: http://www.carlboettiger.info/2012/03/20/citing-r-packages.html

All in all

IMO there are two big issues:

• automatic tests (and the way the code is structured): I'm not sure how to handle automatic tests in the current way the package is structured (rmd instead of functions in a package). IMO it should become a package which has a few API functions which are called by a user and which spit out a html file for a user. These can then be tested. This would also make it a more releaseable package . -> I'm not sure if this is a blocker or not. Someone else should answer that. :-)
• The paper should cite the packages it directly depends on.

[karthik]

@jsalminen Can you please review and respond to the issues raised above?

thank you.

[jsalminen]

@karthik We've discussed about the issues and @aknutas will respond to individual issues.

[aknutas]

Thank you for the feedback, especially with the citation issue. Instructions @janschulz gave us here were very helpful to ensure that the references to other scientific software are up to date.

@karthik, we have addressed the minor and major issues as listed below.

The visual improvement proposals were assigned a responsible person, taken under work and set a future release version (issues aknutas/nails#13, aknutas/nails#14 and aknutas/nails#15).

The major issues were corrected to the best of our ability:

1. The release version was changed to v1.0.0 .
2. We created a start up Linux script for manual testing and GUI-less startup. Changing the software to be an R package would require major refactoring both in the R code and the online web analysis site we provide. However, we discussed the possibility with @jsalminen and are considering using a portion of our upcoming summer vacations to create the package. (issue aknutas/nails#12)
3. Citations to all packages we use were added. (commits https://github.com/aknutas/nails/commit/b77b562430e5fa8f006205dda0fc7bd123a19473 https://github.com/aknutas/nails/commit/33079fc04b12c382b650a0321bfda6e121066139 and https://github.com/aknutas/nails/commit/69842b2a9f201b777d37e591929e7e240db4e9f3)

[karthik]

@jsalminen thank you so much for the thoughtful review, we really appreciate it. And thanks also to @aknutas for the submission and timely responses.

I apologize for not noticing earlier (prior to inviting review) that the submission isn't a software package, but a collection of scripts. As a journal we strongly prefer that software be packaged (and ideally also submitted to a package manager) rather than exist as a collection of notebooks and scripts. This would alleviate issues around automatic testing, and also installation issues (especially with the dependencies). Much of the reviewer checklist is also built around the idea that the submission being reviewed is a package.

However, given that the review is now complete, as a gesture of good will to both the submitting authors and the reviewer we are going to accept this submission but plan to modify our submission guidelines to document that submissions of this kind would not meet our submission criteria in the future.

As @janschulz notes:

but the way the repo is structured for users it is a bit unusual and IMO would work much better as a package which could be installed (pulling in all required dependencies) and then used as a function.

We also agree that this would be better off as an R package. I see that the authors note that this refactoring would be a considerable effort on their part, and not something they will be able to do till the summer (issue aknutas/nails#12). So if the authors are willing, we are also happy to pause the review while they invest the effort in creating an actual R package and then follow through with the acceptance immediately after.

@aknutas: Please let us know your thoughts.

[jsalminen]

Thank you @karthik and @janschulz for the review! The suggestion about building an R package is very insightful and we've decided to go for it. As we're not in a rush with the publication, we figured it is better to do it properly, even if it takes a while longer.

You can pause the review and we'll get back to you once the R package is ready.

[karthik]

@jsalminen Fantastic! I applaud your decision and we will fast track your acceptance as soon as you update this thread. If I can be of any help with the packaging process, please ping me in any of your issues directly.

[aknutas]

Dear editor @karthik ,
@jsalminen and I have created a package version of the software at https://github.com/aknutas/nails-package . It has proper package structure and testthat-based tests that are automatically run by Travis CI. We have also written documentation as man files and in a vignette that explains how to create a report with the package.

Best regards,
Antti Knutas and Juho Salminen

[karthik]

Hi @aknutas,
I will pass this back to @janschulz for additional review. In my quick review I did note some issues.

1. Please __do not__ include a packrat environment inside your package. Declare your dependencies. Packrat is meant for project level use and is not designed to be shipped with packages. Please address this issue asap.
2. Your README does not include any basic intructions on installation. Please add that in, and also link to a vignette.
3. Nice work on adding the codemeta.json

[aknutas]

Dear editor @karthik and reviewer @jankatins ,
thank you for your feedback and the patience for first time package builders. We have now improved the package accordingly.

1. Packrat has now been moved to development branch and the main branch is cleaned and "ready for public". The DESCRIPTION file has now the required version numbering added to the list of dependencies. We have verified that the listed dependencies are sufficient (for example Travis CI pulls required R packages using that).
2. README has now use instructions and links to the vignette. The vignette has been prepared with through usage examples that include sample code and sample output.

Additionally we have automatic test cases for the functions, and they have been set up to be checked by Travis CI automatically.

We hope the improvements are sufficient. This has been a great learning experience and we look forward to instructions for finishing the review process.

Best regards,
Antti Knutas and Juho Salminen

Edit 24.1.2017 by @aknutas - fixed a typo in the reviewer username, properly tagged now

[aknutas]

Dear editor @karthik and reviewer @jankatins ,
would it be possible to unpause the review for this submission?

Thank you in advance.

Best regards,
Antti Knutas and Juho Salminen

[jankatins]

I'll get to it hopefully today. Sorry for the long wait :-(

[jankatins]

I only looked at the issues left open in https://github.com/openjournals/joss-reviews/issues/233#issuecomment-292734519

repository URL

This is now based on https://github.com/aknutas/nails-package, so the mentoned repository URL in the first coment is NOT correct anymore. :question:

version

The package has a 1.0.2 release: https://github.com/aknutas/nails-package/releases/tag/1.0.2 so the version is not the same as the one mentioned in the first comment anymore :-( :question:

automated tests

testthat tests are there and travis is enabled :white_check_mark:

citations

Looks great now! :white_check_mark:

I ran the vignette and this looks nice.

So apart from the change of the repo and version, this looks good. I'm not sure how to solve that problem. @karthik, can you help out with that?

[aknutas]

Thank you for the feedback, @jankatins ! Now that the refactoring is completed, we look forward to implementing additional usability and analysis features in the near future. Your comments and the examples provided will help us retarget efforts for the 1.1.0 release.

We had to change the repository, because the old structure of the R package was very nonstandard and not a package at all. Additionally, we received feedback during the review process that had us updating releases, which is why the release version increased. Could the editor @karthik offer advice regarding this?

[karthik]

@aknutas Sorry I missed this. Not a problem to update the repo. Please edit the issue above to fix the new repo link. Once you do that, please let me know and I'll do a review of the feedback and updates to date. Thanks!

[aknutas]

Thank you for the further information. The new repository link is available at https://github.com/aknutas/nails-package and the release is https://github.com/aknutas/nails-package/releases/tag/1.0.2 . @jankatins can you edit the original issue as I don't have edit access?

[jankatins]

I updated the first comment with a new url and the new version. Not sure what else I can do here.

[karthik]

@aknutas

I found a few more issues that I think you should fix before we accept
```

✖ 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.```

Example: https://github.com/karthik/wesanderson/blob/master/DESCRIPTION#L20

✖ 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.

Example: https://github.com/karthik/wesanderson/blob/master/DESCRIPTION#L21

``` ✖ 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/clean_data.R:108:10

✖ 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/create_author_network.R:10:1
R/create_author_network.R:70:1
R/create_author_network.R:75:1
R/create_author_network.R:76:1
R/create_author_network.R:150:1
... and 49 more lines

✖ avoid sapply(), it is not type safe. It might return a
vector, or a list, depending on the input data. Consider using
vapply() instead.

R/clean_data.R:72:20
R/clean_data.R:157:19
R/clean_data.R:159:17
R/clean_data.R:161:20
R/create_author_network.R:120:20
... and 2 more lines

✖ avoid 1:length(...), 1:nrow(...), 1:ncol(...),
1:NROW(...) and 1:NCOL(...) expressions. They are error prone and
result 1:0 if the expression on the right hand side is zero. Use
seq_len() or seq_along() instead.

R/clean_data.R:77:14
R/create_author_network.R:80:19
R/topic_model.R:89:15

✖ fix this R CMD check WARNING: Found the following file
with a non-portable file name:
tests/testthat/test_data/datafolder/crowdsourcing_old (1).txt These
are not fully portable file names. See section ‘Package structure’
in the ‘Writing R Extensions’ manual.

Don't use file names with spaces in the data folder.

✖ fix this R CMD check WARNING: Found the following file
with non-ASCII characters: read_data.R Portable packages must use
only ASCII characters in their R code, except perhaps in comments.
Use \uxxxx escapes for other characters.

Remove UTF-8 characters

✖ fix this R CMD check WARNING: '::' or ':::' imports not
declared from: ‘devtools’ ‘RCurl’ ‘XML’ Namespaces in Imports field
not imported from: ‘dplyr’ ‘stringi’ All declared Imports should be
used.

Remove any undeclared imports

✖ fix this R CMD check NOTE: clean_wos_data: no visible
binding for global variable ‘fixed_fieldtags’ create_edges: no
visible global function definition for ‘combn’ get_author_edges: no
visible binding for global variable ‘Authors’ get_author_edges: no
visible binding for global variable ‘AuthorFullName’
get_author_nodes: no visible global function definition for
‘aggregate’ get_author_nodes: no visible binding for global
variable ‘Id’ get_author_nodes: no visible binding for global
variable ‘Label’ get_author_nodes: no visible binding for global
variable ‘Freq’ get_author_nodes: no visible binding for global
variable ‘TotalTimesCited’ get_author_nodes: no visible binding for
global variable ‘AuthorAddress’ get_author_nodes: no visible
binding for global variable ‘EmailAddress’ get_author_nodes: no
visible binding for global variable ‘Location’
get_literature_nodes: no visible binding for global variable ‘DOI’
get_literature_nodes: no visible binding for global variable
‘YearPublished’ get_literature_nodes: no visible binding for global
variable ‘ReferenceString’ get_literature_nodes: no visible binding
for global variable ‘id’ get_literature_nodes: no visible binding
for global variable ‘PublicationType’ get_literature_nodes: no
visible binding for global variable ‘AuthorFullName’
get_literature_nodes: no visible binding for global variable
‘DocumentTitle’ get_literature_nodes: no visible binding for global
variable ‘PublicationName’ get_literature_nodes: no visible binding
for global variable ‘BookSeriesTitle’ get_literature_nodes: no
visible binding for global variable ‘Language’
get_literature_nodes: no visible binding for global variable
‘DocumentType’ get_literature_nodes: no visible binding for global
variable ‘ConferenceTitle’ get_literature_nodes: no visible binding
for global variable ‘ConferenceDate’ get_literature_nodes: no
visible binding for global variable ‘ConferenceLocation’
get_literature_nodes: no visible binding for global variable
‘ConferenceSponsors’ get_literature_nodes: no visible binding for
global variable ‘AuthorKeywords’ get_literature_nodes: no visible
binding for global variable ‘SubjectCategory’ get_literature_nodes:
no visible binding for global variable ‘TimesCited’
get_literature_nodes: no visible binding for global variable
‘Abstract’ get_location : : no visible global function
definition for ‘tail’ read_wos_folder: no visible global function
definition for ‘read.delim2’ read_wos_folder: no visible binding
for global variable ‘read.delim2’ read_wos_txt: no visible global
function definition for ‘read.delim2’ topicmodels_json_ldavis: no
visible global function definition for ‘%>%’
topicmodels_json_ldavis: no visible global function definition for
‘stri_count’ Undefined global functions or variables: %>% Abstract
aggregate AuthorAddress AuthorFullName AuthorKeywords Authors
BookSeriesTitle combn ConferenceDate ConferenceLocation
ConferenceSponsors ConferenceTitle DocumentTitle DocumentType DOI
EmailAddress fixed_fieldtags Freq id Id Label Language Location
PublicationName PublicationType read.delim2 ReferenceString
stri_count SubjectCategory tail TimesCited TotalTimesCited
YearPublished Consider adding importFrom("stats", "aggregate")
importFrom("utils", "combn", "read.delim2", "tail") to your
NAMESPACE file.
✖ fix this R CMD check WARNING: Undocumented code objects:
‘fixed_fieldtags’ ‘original_fieldtags’ Undocumented data sets:
‘fixed_fieldtags’ ‘original_fieldtags’ All user-level objects in a
package should have documentation entries. See chapter ‘Writing R
documentation files’ in the ‘Writing R Extensions’ manual.
✖ fix this R CMD check WARNING: Warning: package needs
dependence on R (>= 2.10)
✖ fix this R CMD check WARNING: Warning: package needs
dependence on R (>= 2.10) fix this R CMD check WARNING: Warning:
package needs dependence on R (>= 2.10)
✖ fix this R CMD check NOTE: '::' or ':::' import not
declared from: ‘devtools’ 'library' or 'require' call not declared
from: ‘ggplot2’
✖ fix this R CMD check WARNING: LaTeX errors when creating
PDF version. This typically indicates Rd problems.
✖ fix this R CMD check ERROR: Re-running with no
redirection of stdout/stderr. Hmm ... looks like a package You may
want to clean up by 'rm -rf /tmp/RtmpkunlEX/Rd2pdf759d1c0d9b97'
✖ 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/create_author_network.R:NA:NA```

[arfon]

@jsalminen @aknutas - please respond to @karthik's feedback.

[aknutas]

Dear @karthik and @arfon,
apologies for the delay. After conferring, we have one query about the review process. For acceptance, is it enough that we address the issues mentioned https://github.com/openjournals/joss-reviews/issues/233#issuecomment-378091508; or is it also required to implement the extra features mentioned in https://github.com/aknutas/nails-package/issues/7 and https://github.com/aknutas/nails-package/issues/8 ?

Best regards,
Antti Knutas and Juho Salminen

[jankatins]

No, not from my side, I just reported that because I would find it useful.

[karthik]

@aknutas Just addressing these outstanding issues would be sufficient. 🙏

[aknutas]

Great news. Thank you, @karthik and other editors! I am currently revising the source code to meet the quality standards and my aim is to complete the revisions by the end of September.

[arfon]

How are you getting along here @aknutas?

[karthik]

Hi @aknutas, Since it has been more than 18 months since this submission started and we are still not close to acceptance, I am going to close this. Please feel free to resubmit once you have made revisions to your package. Good luck!