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

roadoi #115

Closed
11 of 14 tasks
njahn82 opened this issue May 17, 2017 · 28 comments
Closed
11 of 14 tasks

roadoi #115

njahn82 opened this issue May 17, 2017 · 28 comments

Comments

@njahn82
Copy link
Member

njahn82 commented May 17, 2017

Summary

  • What does this package do? (explain in 50 words or less):

roadoi interacts with the oaDOI API, a simple web interface that links Digital Object Identifiers (DOIs) and open access full-texts disseminated by both open access journals and repositories.

  • Paste the full DESCRIPTION file inside a code block below:
Package: roadoi
Type: Package
Title: Find Free Versions of Scholarly Publications via the oaDOI Service
Version: 0.2
Authors@R: c(
    person("Najko", "Jahn", role = c("aut", "cre"), email = "[email protected]"))
Description: This web client interfaces oaDOI <https://oadoi.org>, a service finding 
    free full-texts of academic papers by linking DOIs with open access journals and
    repositories. It provides unified access to various data sources for open access
    full-text links including Crossref, Bielefeld Academic Search Engine (BASE) and 
    the Directory of Open Access Journals (DOAJ). API usage is free and no 
    registration is required.
License: MIT + file LICENSE
URL: https://github.com/njahn82/roadoi
BugReports: https://github.com/njahn82/roadoi/issues
LazyData: TRUE
Imports:
    httr,
    jsonlite,
    dplyr,
    plyr,
    miniUI,
    shiny
Suggests:
    roxygen2 (>= 6.0.1),
    testthat,
    knitr,
    covr,
    rmarkdown,
    lubridate,
    rcrossref
RoxygenNote: 6.0.1
VignetteBuilder: knitr

  • URL for the package (the development repository, not a stylized html page):

https://github.com/njahn82/roadoi

  • Which categories does the package fall under from our package fit policies? (e.g., data retrieval, reproducibility. If you are unsure, we suggest you make a pre-submission inquiry.):

data retrieval

  • Who is the target audience?

This package aims at academics who want to find openly available full-texts within R. More specifically, roadoi can be beneficial for text-mining because it retrieves links to full-texts, as well as for those studying scholarly communication. This package is also of practical use for scholarly communication librarians and analysts.

  • Are there other R packages that accomplish the same thing? If so, what is different about yours?

To my knowledge, there aren't any other R packages on CRAN wrapping the oaDOI-API. However, some R packages allow for searching and retrieving scholarly publications. One example, which is maintained by rOpenSci, is fulltext, a wrapper for searching freely available full-texts across many open access sources.

Requirements

Confirm each of the following by checking the box. This package:

  • does not violate the Terms of Service of any service it interacts with.

  • has a CRAN and OSI accepted license.

  • contains a README with instructions for installing the development version.

  • includes documentation with examples for all functions.

  • contains a vignette with examples of its essential functions and uses.

  • has a test suite.

  • has continuous integration, including reporting of test coverage, using
    services such as Travis CI, Coeveralls and/or CodeCov.

  • I agree to abide by ROpenSci's Code of Conduct during
    the review process and in maintaining my package should it be accepted.

Publication options

  • Do you intend for this package to go on CRAN? <- The package is already available via CRAN
  • Do you wish to automatically submit to the Journal of Open Source Software? If so:
    • The package contains a paper.md with a high-level description in the package root or in inst/.
    • The package is deposited in a long-term repository with the DOI:
    • (Do not submit your package separately to JOSS)

Detail

  • Does R CMD check (or devtools::check()) succeed? Paste and describe any errors or warnings:

  • Does the package conform to rOpenSci packaging guidelines? Please describe any exceptions:

  • Please indicate which category or categories from our package fit policies this package falls under and why:

I think this package falls best under the data retrieval category, because it supports the discovery of openly available scholarly publications. The oaDOI service itself claims to index more than 90 million articles covering sources like Crossref, a DOI minting agency, or the Bielefeld Academic Search Engine (BASE).

  • 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:

@sckott
Copy link
Contributor

sckott commented May 18, 2017

thx for your submission @njahn82

editor checks coming soon

@sckott
Copy link
Contributor

sckott commented May 19, 2017

Editor checks:

  • Fit: The package meets criteria for fit and overlap
  • Automated tests: Package has a testing suite and is tested via Travis-CI or another CI service.
  • License: The package has a CRAN or OSI accepted license
  • 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)?

Editor comments

Currently seeking reviewers. It's a good fit and not overlapping.

  • Played a bit with the package:
    • seems like email will take any string - maybe it's worth validating email addresses - seems like they don't server side either, though maybe they are just using it as a string and not validating it
  • Ran goodpractice::gp(), not a lot to deal with. Test coverage is quite good. Could shorten line lengths to tidy things up a bit.
It is good practice towrite unit tests for all functions, and all package code in general. 81% of code lines are covered by test cases.

    R/oadoi_fetch.r:126:NA
    R/oadoi_fetch.r:127:NA
    R/oadoi_fetch.r:128:NA
    R/oadoi_fetch.r:129:NA
    R/oadoi_fetch.r:130:NA
    ... and 4 more linesavoid 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/oadoiAddins.R:24:1

Reviewers: @rossmounce @tts
Due date: 2017-06-12

@njahn82
Copy link
Member Author

njahn82 commented May 20, 2017

Thanks for your suggestions! Email validation is now implemented and the long line is wrapped for better code readability.

@sckott
Copy link
Contributor

sckott commented May 20, 2017

cool, thx

@sckott
Copy link
Contributor

sckott commented May 22, 2017

Reviewers assigned! Big thx to @rossmounce @tts - due date above

@tts
Copy link

tts commented Jun 1, 2017

Package Review

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • Installation instructions: for the development version of package and any non-standard dependencies in README
  • Vignette(s) demonstrating major functionality that runs successfully locally
  • Function Documentation: for all exported functions in R help
  • Examples for all exported functions in R Help that run successfully locally
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING, and URL, Maintainer and BugReports fields in DESCRIPTION

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests: Unit tests cover essential functions of the package
    and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 5


General Comments

This is a compact and useful package. For me personally timing was perfect because I had plans to add links to open access fulltext to my latest altmetrics Shiny app aaltovirta. Now links are in place, thanks to your work, @njahn82!

Although I do not really belong to the RStudio addin focus group, I can imagine that there are many people who welcome this functionality too.

Tests

devtools::check() and devtools::test() ran without errors on Ubuntu 16.04.2 LTS, R 3.2.3.

The RStudio addin works as expected.

Documentation

  • In oadoi_fetch.R, line 3: acccess --> access
  • In oadoi_fetch.R, line 12: adress --> address
  • In oadoi_fetch.R, lines 24-57: the following are returned by the function but missing from here: _closed_base_ids, _closed_urls, oa_color_long
  • In oadoi_fetch.R, lines 24-57: columns are sorted by name which is fine. However, I wonder if the preceding column numbering is needed at all because as of now, it does not reflect the order of the result tibble. Or is it so that the .Rd command \tabular does not serve very well here?
  • In oadoi_fetch.R, line 34: green_base_collections --> _green_base_collections
  • In vignette intro.Rmd, line 27: json --> JSON

@sckott
Copy link
Contributor

sckott commented Jun 1, 2017

thanks for your review @tts !

@sckott
Copy link
Contributor

sckott commented Jun 7, 2017

@rossmounce just a friendly note that your review is due next week, thx 😸

@rossmounce
Copy link

@sckott thanks for the heads up. I've started to look into the package but I'll probably need a few more days (perhaps?).

@sckott
Copy link
Contributor

sckott commented Jun 12, 2017

Okay, thanks @rossmounce

@rossmounce
Copy link

rossmounce commented Jun 15, 2017

Package Review

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • Installation instructions: for the development version of package and any non-standard dependencies in README
  • Vignette(s) demonstrating major functionality that runs successfully locally
  • Function Documentation: for all exported functions in R help
  • Examples for all exported functions in R Help that run successfully locally
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING, and URL, Maintainer and BugReports fields in DESCRIPTION

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests: Unit tests cover essential functions of the package
    and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 4 hours


Review Comments

General Comments

This is an excellent wrapper for the R universe, for an excellent API and an excellent set of data resources beneath it all. It's really impressive to see that with tools like this, open access is really starting to come together and be more of a seamless and universal user-experience.

As a new OA grants manager, I'm hugely interested in tools that'll make my life easier/quicker/more accurate when attempting to evaluate the open access compliance of our grantees work. I will be sure to use this package in my compliance workflow.

Furthermore, I might discuss this with other open access administrators on the next open research funders group (ORFG) call. There are a lot of smaller nonprofits and charities out there that will need a free, open-source software solution to help them with compliance checking of outputs so I'm really glad this package exists.

Once I get to use this package more, I'm sure I'll have more comments/issues but for the moment I just want to get this review done so it isn't a blocker.

Tests

devtools::check() and devtools::test() ran without errors on Ubuntu 16.04.2 LTS, R 3.2.5. When I get permission to install Rstudio on my 'work' machine, I will let you how the tests run under Win10.

I did not try the RStudio addin.

Documentation

In vignettes/intro.Rmd, line 68: when you are tired to type in your email --> when you are too tired to type in your email

Installation via devtools & github was smooth, but I ran into a small problem when trying to install via CRAN which I reported as an issue: ropensci/roadoi#10 Perhaps there should be a dependencies/pre-requisites section in the README so that new users might be made more aware of what they need before attempting to install roadoi ?

I was intrigued by what "blue OA" was, and I still have no concrete idea of what it is. I reported this as an issue and am waiting to hear back from Impactstory about it: ropensci/roadoi#11

@sckott
Copy link
Contributor

sckott commented Jun 15, 2017

thanks for your @rossmounce !

@njahn82 continue discussion here - can also ping editors with any questions

@njahn82
Copy link
Member Author

njahn82 commented Jun 16, 2017

Thank you @tts @rossmounce for your very kind and helpful reviews. I will post my response within the next two weeks

@njahn82
Copy link
Member Author

njahn82 commented Jun 23, 2017

I have now addressed the reviewer concerns, thanks again to @tts and @rossmounce for taking the time to review the package and for your very helpful suggestions. I am so glad to read that you not only see potential use of this package for your work, but that you have already used it.

All changes made to the source code can be found here:
ropensci/roadoi#14

@tts comments and suggestions:

In oadoi_fetch.R, line 3: acccess --> access

Fixed.

In oadoi_fetch.R, line 12: adress --> address

Fixed.

In oadoi_fetch.R, lines 24-57: the following are returned by the function but missing from here: _closed_base_ids, _closed_urls, oa_color_long

Added. I also added a comment that, according to the API documentation, return fields are not fixed and some of them may be added or removed by oaDOI

In oadoi_fetch.R, lines 24-57: columns are sorted by name which is fine. However, I wonder if the preceding column numbering is needed at all because as of now, it does not reflect the order of the result tibble. Or is it so that the .Rd command \tabular does not serve very well here?

Agreed. I removed the numbering in the table.

In oadoi_fetch.R, line 34: green_base_collections --> _green_base_collections

Fixed.

In vignette intro.Rmd, line 27: json --> JSON

Fixed.

@rossmounce comments and suggestions:

In vignettes/intro.Rmd, line 68: when you are tired to type in your email --> when you are too tired to type in your email

Fixed

Installation via devtools & github was smooth, but I ran into a small problem when trying to install via CRAN which I reported as an issue: ropensci/roadoi#10 Perhaps there should be a dependencies/pre-requisites section in the README so that new users might be made more aware of what they need before attempting to install roadoi ?

Thank you for making me aware of this dependency. I now added the required version of the shiny package to the DESCRIPTION file as dependency.

I was intrigued by what "blue OA" was, and I still have no concrete idea of what it is. I reported this as an issue and am waiting to hear back from Impactstory about it: ropensci/roadoi#11

Agreed. The term blue open access is rather uncommon and the meaning is not clear. I am still waiting for an answer from the Impactstory team and will add it to the documentation.

Because of your valuable input, I would like to add you as contributors to the DESCRIPTION. Are you ok with that?

@noamross
Copy link
Contributor

@njahn82 re: adding contributors, we have a little experiment on that front. So as not to clutter up this review thread, can you take a look at ropensci/software-review-meta#1 ?

@tts
Copy link

tts commented Jun 26, 2017

I'm ok with that. Thanks for asking @njahn82

@rossmounce
Copy link

I'd be delighted to be listed as a contributor. Thanks @njahn82

@sckott
Copy link
Contributor

sckott commented Jun 27, 2017

Thanks for making changes @njahn82

@tts and @rossmounce - Are you happy with the changes/fixes? Any other concerns/thoughts you have?

@tts
Copy link

tts commented Jun 28, 2017

Quite happy with fixes from my part.

One thing I didn't check earlier was the behaviour of the Done button in the RStudio addin. At the moment, it doesn't respond to a click. AFAIK you need to add a listener to input$done Something like

shiny::observeEvent(input$done, {
      stopApp()
    })

as explained here

From my current (default) addins, Add Crossref Citations wraps stopApp with invisible but I'm not sure if that's necessary.

@njahn82
Copy link
Member Author

njahn82 commented Jun 28, 2017

Very helpful @tts, I fixed the bug as suggested.

@sckott
Copy link
Contributor

sckott commented Jun 28, 2017

anything else @rossmounce ?

@rossmounce
Copy link

Nope. Happy with the changes/fixes @sckott

@sckott
Copy link
Contributor

sckott commented Jul 14, 2017

Approved! thank for your submission @njahn82 !

  • Please transfer the package to ropensci- I've invited you to a team within our ropensci organization. After you accept you should be able to move it.
  • Make sure to
    • add rOpenSci footer to README
      [![ropensci_footer](https://ropensci.org/public_images/ropensci_footer.png)](https://ropensci.org)
    • Add the (brand new!) "Peer-reviewed" badge to your README (in the future people will add this on submittal; it updates through the stages of review):
      [![](https://ropensci.org/badges/115_status.svg)](https://github.com/ropensci/onboarding/issues/115)
    • Change any needed links, such those for CI badges
    • Travis CI should update to the new location automatically - you may have to update other CI systems manually
  • Are you interested in doing a blog post for our blog https://ropensci.org/blog/ ? either a short-form intro to it (https://ropensci.org/tech-notes/) or long-form post with more narrative about its development (https://ropensci.org/blog/). If so, i'll get you the details on that

@njahn82
Copy link
Member Author

njahn82 commented Jul 16, 2017

Great, thank you very much for accepting this package. I am very grateful about the reviews, which were very helpful to improve this package!

I transferred the repo as described. After adding the "peer-review" badge, I get the following warning when calling R CMD Check --as-cran

* checking top-level files ... WARNING
Conversion of ‘README.md’ failed:
pandoc: Could not fetch https://ropensci.org/badges/115_status.svg
HttpExceptionRequest Request {
  host                 = "ropensci.org"
  port                 = 443
  secure               = True
  requestHeaders       = []
  path                 = "/badges/115_status.svg"
  queryString          = ""
  method               = "GET"
  proxy                = Nothing
  rawBody              = False
  redirectCount        = 10
  responseTimeout      = ResponseTimeoutDefault
  requestVersion       = HTTP/1.1
}
 (InternalException (HandshakeFailed Error_EOF))

Can you help @sckott ?

@njahn82
Copy link
Member Author

njahn82 commented Jul 16, 2017

My fault, I was running pandoc 1.18, which caused this warning. Upgrading to the newest pandoc release helps.

@njahn82
Copy link
Member Author

njahn82 commented Jul 16, 2017

Are you interested in doing a blog post for our blog https://ropensci.org/blog/ ? either a short-form intro to it (https://ropensci.org/tech-notes/) or long-form post with more narrative about its development (https://ropensci.org/blog/). If so, i'll get you the details on that

Sounds good. Will prepare a draft and will send it to you by mail within the next two weeks.

@njahn82
Copy link
Member Author

njahn82 commented Jul 17, 2017

Is on CRAN now!

@sckott
Copy link
Contributor

sckott commented Jul 17, 2017

thanks!

@sckott sckott closed this as completed Jul 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants