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

RefManageR #119

Closed
mwmclean opened this issue Jun 2, 2017 · 35 comments

Comments

@mwmclean
Copy link

commented Jun 2, 2017

Summary

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

Provides tools for importing and working with bibliographic references. Greatly enhances the bibentry class in R by providing a class BibEntry which stores BibTeX and BibLaTeX references, and can be easily searched by any field, by date ranges, and by various formats for name lists using R's person class.

  • Paste the full DESCRIPTION file inside a code block below:
Package: RefManageR
Version: 0.14.3
Date: 2017-06-02
Title: Straightforward 'BibTeX' and 'BibLaTeX' Bibliography Management
Authors@R: person(c("Mathew", "W."), "McLean", role = c("aut", "cre"),
    email = "mathew.w.mclean@gmail.com")
Author: Mathew W. McLean [aut, cre]
Maintainer: Mathew W. McLean <mathew.w.mclean@gmail.com>
Description: Provides tools for importing and working with bibliographic
    references. It greatly enhances the 'bibentry' class by providing a class
    'BibEntry' which stores 'BibTeX' and 'BibLaTeX' references, supports 'UTF-8'
    encoding, and can be easily searched by any field, by date ranges, and by
    various formats for name lists (author by last names, translator by full names,
    etc.). Entries can be updated, combined, sorted, printed in a number of styles,
    and exported. 'BibTeX' and 'BibLaTeX' '.bib' files can be read into 'R' and
    converted to 'BibEntry' objects. Interfaces to 'NCBI Entrez', 'CrossRef', and
    'Zotero' are provided for importing references and references can be created
    from locally stored 'PDF' files using 'Poppler'. Includes functions for citing
    and generating a bibliography with hyperlinks for documents prepared with
    'RMarkdown' or 'RHTML'.
License: GPL-2 | GPL-3 | BSD_3_clause + file LICENSE
Imports:
    xml2,
    jsonlite,
    utils,
    plyr,
    tools,
    httr,
    bibtex,
    lubridate (>= 1.5.0),
    stringr,
    methods
Suggests:
    knitr,
    testthat
Encoding: UTF-8
Depends:
    R (>= 3.0)
VignetteBuilder: knitr
BugReports: https://github.com/mwmclean/RefManageR/issues
RoxygenNote: 6.0.1
  • URL for the package (the development repository, not a stylized html page):

https://github.com/mwmclean/RefManageR

  • Please indicate which category or categories from our [package fit policies]

Reproducibility because it provides functions as well as vignette examples of working together with knitr to include references in reproducible reports. Data access because of the functions it provides for importing bibliographic information into R.

  • Who is the target audience?

Anyone that wants to write reproducible reports or simply organize bibliographic data from a variety of sources.

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

No package I'm aware of provides all the functionality, but there is some overlap with other packages.
Base R provides the bibentry class. RefManageR provides a class BibEntry that extends bibentry in a number of ways, such as support for BibLaTeX; improved handling of name list fields (author, editor, etc.); improved handling of dates (using lubridate); additional options for sorting, formatting, and printing; and additional methods that make it easier to search, sort, and modify parts of the BibEntry object. There is a small overlap with the packages rentrez and rcrossref, in that RefManageR can make use of the same APIs that those packages use to get references. However, RefManageR parses the results differently and returns a BibEntry object.

Requirements

Aside from the packages and version of R listed above in the DESCRIPTION, poppler is needed if a user want to extract bibliographic information from PDF files on their machine.

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? (Has already been on CRAN for a few years)
  • 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)

Note: I already submitted to JOSS here

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:

Exceptions: I did not follow your guidelines when naming the functions in my package since they were named long ago and have been on CRAN for a long time now (I used title case and no underscores).

  • 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:
    @karthik @cboettig @sckott

@ottlngr ottlngr referenced this issue Jun 6, 2017
11 of 14 tasks complete
@noamross noamross self-assigned this Jun 8, 2017
@noamross

This comment has been minimized.

Copy link
Collaborator

commented Jun 11, 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

Thank you for your submission, @mwmclean! We were actually in the midst of revisions of our Aims and Scope when you submitted this, and one of our updates was to explicitly add reference management as a component of our reproducibility category.

Below are the results of our automated tests. I don't see anything here to prevent review going forward, but we will want these addressed before acceptance. This also provides a reference for reviewers of areas they may want to pay attention to. If you make changes in response to these or have a written response, please note them on the thread here so reviewers know about updates. I am currently seeking reviewers.

>goodpractice::gp()
── GP RefManageR ─────────────────────────────────

It is good practice to

  ✖ write unit tests for all functions, and all package
    code in general. 66% of code lines are covered by test
    cases.

    R/02BibOptions.R:119:NA
    R/02BibOptions.R:120:NA
    R/04InternalFunctions.R:16:NA
    R/04InternalFunctions.R:17:NA
    R/04InternalFunctions.R:18:NA
    ... and 1794 more lines

  ✖ write short and simple functions. These functions have
    high cyclomatic complexity:GetFormatFunctions (130),
    MakeBibLaTeX (97), Cite (83), MakeAuthorYear (77),
    toBibtex.BibEntry (63), ResolveBibLaTeXCrossRef (58).
  ✖ 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.
  ✖ 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/BibEntryAddOp.R:52:19
    R/ReadGS.R:63:20
    R/ReadPDFsSupport.R:130:20
    R/ReadPubMed.R:60:21
    R/rmdCite.R:208:17

  ✖ 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/02BibOptions.R:111:1
    R/02BibOptions.R:133:1
    R/02BibOptions.R:143:1
    R/02BibOptions.R:144:1
    R/02BibOptions.R:145:1
    ... and 785 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/04InternalFunctions.R:13:11
    R/04InternalFunctions.R:56:11
    R/04InternalFunctions.R:375:13
    R/04InternalFunctions.R:385:15
    R/04InternalFunctions.R:386:45
    ... and 79 more lines

  ✖ not import packages as a whole, as this can cause name
    clashes between the imported packages. Instead, import
    only the specific functions you need.
  ✖ fix this R CMD check NOTE: Foreign function call to a
    different package: .External("do_read_bib", ...,
    PACKAGE = "bibtex") See chapter ‘System and foreign
    language interfaces’ in the ‘Writing R Extensions’
    manual.
───────────────────────────────────

Running your tests, I get the following messages, plus several more of the locale-based warnings.

Skipped ------------------------------------------------------------------------
1. Reading one PDF file name (@test-readPDF.R#68) - Couldn't copy jstor.pdf

2. Recognizes JSTOR (@test-readPDF.R#77) - Couldn't download Poppler or JSTOR PDF

Warnings -----------------------------------------------------------------------
1. PrintBibliography when multiple BibEntry objects cited (@test-cites.R#131) - input string 8 is invalid in this locale

2. Creates a BibEntry object (@test-readPDF.R#54) - back-tracking limit reached in PCRE for element 1

3. Creates a BibEntry object (@test-readPDF.R#54) - back-tracking limit reached in PCRE for element 1

4. Creates a BibEntry object (@test-readPDF.R#54) - input string 1 is invalid in this locale

In addition, devtools::spell_check() yields these (in addition to many other non-dictionary words):

simplying           SearchBib.Rd:48
requrested          ReadPDFs.Rd:34
mutliple            c.BibEntry.Rd:18
invisibling         head.BibEntry.Rd:33
gerenated           RefManageR-package.Rd:46
exctract            SearchBib.Rd:17, sub-sub-.BibEntry.Rd:12
decribed            print.BibEntry.Rd:49
coereced            sub-subset-.BibEntry.Rd:15

Reviewers: @cboettig @AmeliaMN
Due date: 2017-07-15

@mwmclean

This comment has been minimized.

Copy link
Author

commented Jun 13, 2017

Thanks for the comments. Regarding the last two notes from goodpractice::gp(), fixing them depends on the version of the bibtex package on GitHub. However, I've been unable to contact the maintainer, @romainfrancois, to submit the updated version of bibtex to CRAN, and hence can't get rid of the NOTE.

@romainfrancois

This comment has been minimized.

Copy link

commented Jun 13, 2017

sorry about that @mwmclean I'll have a look

@noamross

This comment has been minimized.

Copy link
Collaborator

commented Jun 24, 2017

Reviewers assigned: @cboettig @AmeliaMN. Due date 2017-07-15

@cboettig

This comment has been minimized.

Copy link
Member

commented Jun 27, 2017

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • 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 DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).

No CONTRIBUTING document or section of the Readme.

Paper (for packages co-submitting to JOSS)

The package contains a paper.md with:

  • A short summary describing the high-level functionality of the software
  • Authors: A list of authors with their affiliations
  • A statement of need clearly stating problems the software is designed to solve and its target audience.
  • References: with DOIs for all those that have one (e.g. papers, datasets, software).
    These do not lterally appear in paper.md, though paper.md uses pandoc-style citations and includes a .bib, so

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


Review Comments

Overview: The RefManageR package is overall a well designed and carefully engineered package that already sees significant use as indicated by the download statistics. (Indeed, I've used the package myself both as a dependency and for occasional use. The source code is easily readible, well documented, and has recently migrated to the httr / curl / xml2 stack. My only real comments are some big picture conceptual things that are no doubt beyond the scope of changes for onboarding, but since I have no more immediate critique to offer I just put them down here as food for thought. Overall great package and nicely done.

The package aims to provide functions that fall into at least three separate buckets, which from the perspective of a developer, might have made more sense for these to be separated out into more complete, separate module packages. That could streamline maintenance and make it easier for other developers to just depend on the parts they need. More importantly, it could help the package provide somewhat more complete implementations of these separate areas, and enforce a bit better definition of a low-level interface that a developer could use to leverage any part. This will make more sense in terms of specifics, which I discuss below. However, I suspect breaking up the package is beyond the scope here and should not prevent onboarding, but merely as a suggestion to the author and something to keep in mind for future directions.

The separate modules, as I see them, are:

  • BibLaTex utilities: define a native R S3 class (BibEntry) corresponding to the BibLaTeX format, along with the ability to parse and serialize these files (much as bibtex package does does for bibtex). This is a well-defined feature set that could easily stand alone, and be useful to others wanting a more fully-featured bibtex class. It might be useful to have these in a seperate BibLatex package that is pulled in by RefManageR.

  • Query and format citations in RMarkdown, etc. These are basically some methods and functions for the BibEntry class that are useful for authors formatting references from R, which is a somewhat narrower application than the more general-purpose idea of providing a somewhat richer native bibliographic format to R. These are the main user-facing functions of RefManageR. These are really well-designed functions, and a nice compliment to the BibEntry that RefManageR provides.

  • Query various external bibliographic formats. The package supports calls to several different platforms (CrossRef, Zotero, NCBI) to import bibliographic metadata. This is a useful ability to be sure, but is easily isolated from the above functions.

Unfortunately, I think separating out these elements also highlights some possible limitations of this design:

  • With regards to the first 'module' of tasks, BibLatex is not an ideal format for providing a richer metadata model than R's native bibentry (which is basically bibtex). It's not easy to distinguish a biblatex from a bibtex file, and most academic publishers do not provide or accept BibLatex format. The similarity to vanilla bibtex seems likely to create a source of confusion for users who may fail to notice that biblatex files are not always bibtex compatible.

  • A consequence of this choice impacts the third 'module' of tasks, querying external bibliographic providers. These queries all seem to request data in bibtex format, which of course simplifies the programming, but also loses much of the data that could be available from the richer metadata formats (Something that is not obvious from the package documentation). For instance, CrossRef/DataCite can tell us that a citation is a piece of software rather than an article or "misc", but this information is lost when we query their API by requesting bibtex. This also defeats the advantage of having a native R format that was any richer than bibtex to begin with.

To me, these seem a bit like missed opportunities in the design. The core "RefManageR" features in section 2 are excellent, but are restricted to only manipulating data in the biblatex format, and only accessing remote data in the bibtex format, which diminishes the potential somewhat. The lack of more modular abstraction also encumbers the source code a little: for instance, this package makes no use of the existing (ropensci) rcrossref implementation, while it's native implementation makes rather heavy use of try calls for error handling, rather than the native http error-code handling apready built into htt (e.g. see httr::stop_for_status())

@mwmclean

This comment has been minimized.

Copy link
Author

commented Jun 28, 2017

Thank you, @cboettig, for the quick and helpful report. I will respond more completely to your report very soon, but for now just wanted to mention a couple updates and sticking points.

  • Regarding the goodpractice checks, I now have the majority of them corrected. The exceptions are
    • the last two already mentioned that are dependent on @romainfrancois submitting bibtex to CRAN
    • the note about cyclomatic complexity. The first three functions listed define several small functions used for formatting entries for printing. These functions are then returned in an environment that gets used when formatting, so I feel like these are false positives. Will have a closer look at the last two functions listed.
  • I have corrected the spelling mistakes in the documentation
  • I am unable to reproduce the warnings mentioned from running the tests regarding "back-tracking limit reached in PCRE"
  • I have just added a CONTRIBUTING.md file
  • I thought I had included references properly using paper.bib, please let me know if I need to change it or paper.md in some way
@romainfrancois

This comment has been minimized.

Copy link

commented Jun 30, 2017

@mwmclean I'm about to upload bibtex 0.4.1 (what is on GitHub now)

@noamross

This comment has been minimized.

Copy link
Collaborator

commented Jul 7, 2017

Hi @AmeliaMN, a friendly reminder that your review is due in one week.

@AmeliaMN

This comment has been minimized.

Copy link

commented Jul 17, 2017

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • 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

The images in the TestHTML vignette aren't appearing for me. Otherwise, the vignettes look good. Related to @cboettig's comments about the three purposes of this package, the vignettes seem to cover mostly the second task. It might be useful to have a vignette illustrating the external querying functionality.

  • Function Documentation: for all exported functions in R helper

I noticed a possible spelling error in the title for the documentation on GetPubMedByID(). I think it should say E-Utilities rather than Entrez.

  • Examples for all exported functions in R Help that run successfully locally

The example for GetBibEntryWithDOI() includes the function http_error(), which is in httr, and not automatically loaded by RefManageR. A variety of other examples use this function (GetDOIs, GetPubMedByID, GetPubMedRelated, LookupPubMed, etc) so it might be worthwhile to import the function, or include the library call in the example.

When I edited the code to include httr::http_error(), I was able to get the example to run, but I got several warning messages:

Warning messages:
1: In parse_Rd(Rd, encoding = encoding, fragment = fragment, ...) :
  <connection>:7: unexpected END_OF_INPUT '
'
2: In parse_Rd(Rd, encoding = encoding, fragment = fragment, ...) :
  <connection>:7: unexpected END_OF_INPUT '
'

The example for GetPubMedByID() returns an warning I don't understand:

Warning message:
In ProcessDate(bib[["year"]], bib[["month"]]) : 

The example for ReadCrossRef() returns an error for this part of the example: ReadCrossRef(filter = list(prefix = "10.5555"), limit = 5, min.relevance = 0),

server error for doi “http://dx.doi.org/10.5555/cs3112-testdoi-4”, you may want to try again.

and part: ReadCrossRef(query = 'rj carroll measurement error', limit = 2, sort = "relevance", min.relevance = 80, use.old.api = TRUE)

server error for doi “http://dx.doi.org/10.1002/0470011815.b2a03082”, you may want to try again.

I didn't see examples for head(), PrintBibliography(), NoCite(), ReadPDFs() or tail().

  • Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).
Paper (for packages co-submitting to JOSS)

The package contains a paper.md with:

  • A short summary describing the high-level functionality of the software
  • Authors: A list of authors with their affiliations
  • A statement of need clearly stating problems the software is designed to solve and its target audience.
  • References: with DOIs for all those that have one (e.g. papers, datasets, software).
    Like Carl, I see references in the text but no reference list literally in the .md file. Not sure if this matters to the editors.

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


Review Comments

This is my first time doing a package review for rOpenSci, but I think this package looks great. RefManageR provides a variety of helpful functions for managing and citing bibliographic entries in RMarkdown. I was not previously aware of the package, but I will certainly be using it in my own work going forward. All the automated checks passed on my system, and the documentation is clear with thorough examples. As noted above, I ran in to a few warnings I didn't understand, but no errors or bugs.

@mwmclean

This comment has been minimized.

Copy link
Author

commented Jul 18, 2017

Thank you for the helpful comments @AmeliaMN. I think I've managed to address the issues you raised with some commits today up to this one.

  • Regarding vignettes:
    • I removed the plot from the "TestHtml" vignette.
    • I feel the "manual" vignette already covers all three areas mentioned above in detail, with Section 3 of that vignette covering the external querying portion of the package.
  • Regarding documenation:
    • I corrected the documentation "E-Utilities" typo mentioned
    • I added httr:: whenever http_error is called in the documentation examples
    • The warning message in the GetBibEntryWithDOI example was due to not decoding the URLs in the returned BibTeX and has been corrected
    • The GetPubMedByID example warning was fixed
    • I changed the two mentioned ReadCrossRef examples so they would not result in those messages about server errors and also changed the message to be more informative about what was happening
    • I added examples to the documentation for head, PrintBibliography, NoCite, ReadPDFs, and tail
@noamross

This comment has been minimized.

Copy link
Collaborator

commented Jul 23, 2017

Thanks for your reviews @cboettig and @AmeliaMN, and your follow-ups @mwmclean. Let me just jump to ask @mwmclean to please let us know when you think your updates have addressed the all the reviewer comments thus far (including commenting here on big-picture stuff). Then we can take a second look at all the changes in aggregate.

Also, your paper.md + paper.bib look fine to me. JOSS will let us know if there's any issue in compiling when we transfer over.

@mwmclean

This comment has been minimized.

Copy link
Author

commented Jul 25, 2017

In response to the goodpractice::gp checks, I have reduced the cyclomatic complexity of the flagged functions, except for the three I mentioned which I feel are false positives. I have now also removed the check NOTE about foreign function call and also the import of bibtex thanks to that package's recent updates. Thus, I feel I have finished responding to the initial checks.

Regarding the comments of @cboettig

  • I have now removed all the try calls wrapping my API queries and instead use httr::http_error and httr::stop_for_status
  • I think the idea of splitting the package into three separate packages for each of the mentioned "buckets" is interesting; I will open an issue for it, but I don't think I'll be able to get to this in the short term
  • Some thoughts regarding BibLaTeX not being ideal:
    1. I don't believe the format returned by the various APIs considered in the package is standardized to the point where we can say there is one clear ideal choice, as it seems each has to be parsed on a case by case basis. For example, the software case you mention for CrossRef/DataCite won't carry over to Zotero as far as I know
    2. There are several instances in the package where additional information returned by the API query is stored in a custom field (or e.g. the "note" field), which the user can easily access/manipulate (bucket 1) or format/print using a custom style they create (bucket 2)
    3. For storing bibliographic information for use with LaTeX, BibLaTeX seems to be the gold standard and because LaTeX is so widely used by statisticians and data scientists, I feel it is a good choice
    4. Since BibTeX is still more widely used by academic publishers, I provide functions to convert from BibLaTeX to BibTeX format and these are explained in the documentation and vignette

@noamross, let me know if I've missed anything, but I believe I've now addressed all the comments thus far.

@noamross

This comment has been minimized.

Copy link
Collaborator

commented Jul 25, 2017

Thank you, @mwmclean. @cboettig and @AmeliaMN, please let us know if the updates have addressed all of your concerns for your reviews. If so, say so below and check off remaining boxes on your reviews. If not, please give details.

@cboettig

This comment has been minimized.

Copy link
Member

commented Jul 25, 2017

Yup, 👍 , I think this is ready to go and I think i've checked all my boxes above.

@AmeliaMN

This comment has been minimized.

Copy link

commented Jul 26, 2017

Looking good! I'm still confused about the Entrez/E-Utilities thing, but I think that's just my lack of familiarity with NCBI.

Thanks for adding those additional examples. The only thing I can see now is the example for ReadPDFs isn't working because

Error in ReadPDFs(path) : poppler does not seem to be installed
@noamross

This comment has been minimized.

Copy link
Collaborator

commented Jul 26, 2017

I believe it's in the help file for ReadPDFs(), but may I suggest amending the error message so it reads

Poppler does not seem to be installed.
ReadPDFs requires that the `pdfinfo` utility from Poppler PDF be installed. 
See http://poppler.freedesktop.org/
@mwmclean

This comment has been minimized.

Copy link
Author

commented Jul 27, 2017

@noamross, thanks for the suggestion; this is now done.

@AmeliaMN

This comment has been minimized.

Copy link

commented Jul 27, 2017

Perfect! I've checked off all my boxes.

@noamross

This comment has been minimized.

Copy link
Collaborator

commented Jul 27, 2017

Approved! Thanks @mwmclean for submitting and @cboettig and @AmeliaMN for your reviews!

To-dos:

  • Transfer the repo to the rOpenSci organization under "Settings" in your repo. I have invited you to a team that should allow you to do so. You'll be made admin once you do.
  • Add the rOpenSci footer to the bottom of your README
[![ropensci_footer](https://ropensci.org/public_images/ropensci_footer.png)](https://ropensci.org)
  • Add the (brand new!) "Peer-reviewed" status badge to your README (in the future people will add this on submittal; it updates through the stages of review):
[![](https://badges.ropensci.org/120_status.svg)](https://github.com/ropensci/onboarding/issues/120)
  • Fix any links in badges for CI and coverage to point to the ropensci URL. (We'll turn on the services on our end as needed)
  • Activate Zenodo watching the repo
  • Tag and create a release so as to create a Zenodo version and DOI
  • Submit to JOSS using the Zenodo DOI. We will tag it for expedited review.

Welcome aboard! We'd also love a blog post about your package, 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/). Let us know if you are interested, and if so @stefaniebutland will be in touch about it when she's back from vacation late next week.

@mwmclean

This comment has been minimized.

Copy link
Author

commented Jul 27, 2017

Thank you very much @noamross, @AmeliaMN, and @cboettig for helping improve the package!! All the To-dos should be done now.

@mwmclean

This comment has been minimized.

Copy link
Author

commented Aug 3, 2017

@noamross do you still need to turn on coverage services for the transferred repo? I've tried setting up both Codecov and Coveralls with no luck.

@noamross

This comment has been minimized.

Copy link
Collaborator

commented Aug 3, 2017

There's an issue with permissions in the transfer to Codecov that we're figuring out that has held up the past couple of migrations. Working on it! Right now it looks like your .travis.yml sends to Codecov (which is not running), but your current badge is pointing to Coveralls. I'll let you know when when have the codecov migration working.

@mwmclean

This comment has been minimized.

Copy link
Author

commented Aug 3, 2017

I switched back to Coveralls. Not sure how long it takes to update, but coveralls.io is still saying "this repo has not received any jobs yet".

@sckott

This comment has been minimized.

Copy link
Member

commented Aug 25, 2017

hi @mwmclean - coveralls is turned on now for this repo, can you try again?

@mwmclean

This comment has been minimized.

Copy link
Author

commented Aug 25, 2017

@sckott I restarted a build on travis just after I saw your message. Still saying no builds on coveralls.io.

@sckott

This comment has been minimized.

Copy link
Member

commented Aug 25, 2017

So i'm not that familiar with coveralls for R pkgs. It's turned on for your repo, but do you need a Coveralls key on Travis as an env var? Used to need one for codecov, but now just connects automatically.

@mwmclean

This comment has been minimized.

Copy link
Author

commented Aug 28, 2017

Looks like something weird with my package or covr. One of my tests is only failing during covr::coveralls(type = "all") even though the build is successful.

@sckott

This comment has been minimized.

Copy link
Member

commented Aug 28, 2017

@jimhester

This comment has been minimized.

Copy link

commented Aug 28, 2017

To debug this you should run

Rscript -e 'covr::package_coverage(type = "all", quiet = FALSE, clean = FALSE)'

Then you can inspect the package and libraries that are created locally.

When I do this I get the following output.

Error: identical(names(y), names) is not TRUE
Execution halted
@stefaniebutland

This comment has been minimized.

Copy link

commented Aug 30, 2017

Hello @mwmclean. Following up to ask if you are interested in writing a blog post about your package: 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/).

@mwmclean

This comment has been minimized.

Copy link
Author

commented Sep 2, 2017

Hi @stefaniebutland, it doesn't look like I'm going to be able to do a blog post in the short term, but I'd like to maybe do one at some point in the future.

@stefaniebutland

This comment has been minimized.

Copy link

commented Sep 5, 2017

Just ping me if and when you're interested and have the time @mwmclean. Thank you for submitting your package!

@sckott

This comment has been minimized.

Copy link
Member

commented Oct 2, 2017

can this be closed?

@stefaniebutland

This comment has been minimized.

Copy link

commented Oct 2, 2017

ok with me to close re: blog post. @mwmclean please ping me if and when you want to do one

@mwmclean

This comment has been minimized.

Copy link
Author

commented Oct 18, 2017

Sorry, did we need my input on the closing question? I still need to track down my issue with covr, but I'm okay to close. I will be sure to message if I get the time to write a blog post.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.