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

oai - an OAI-PMH client #19

Closed
sckott opened this Issue Aug 21, 2015 · 8 comments

Comments

Projects
None yet
3 participants
@sckott
Member

sckott commented Aug 21, 2015

    1. What does this package do? (explain in 50 words or less)

oai is an R client to work with OAI-PMH services, used by many libraries and other content distributors (e.g. Dryad, Datacite, etc.). The data is metadata for an object (book, video, digital files, etc.)

    1. Paste the full DESCRIPTION file inside a code block (bounded by ``` on either end).
Package: oai
Type: Package
Title: General Purpose 'Oai-PMH' Services Client
Description: A general purpose client to work with any 'OAI-PMH'
    service. The 'OAI-PMH' protocol is described at
    http://www.openarchives.org/OAI/openarchivesprotocol.html.
    Functions are provided to work with the 'OAI-PMH' verbs: 'GetRecord',
    'Identify', 'ListIdentifiers', 'ListMetadataFormats', 'ListRecords', and
    'ListSets'.
Version: 0.0.5.9000
License: MIT + file LICENSE
Authors@R: c(person("Scott", "Chamberlain", role = c("aut", "cre"),
    email = "myrmecocystus@gmail.com"))
URL: https://github.com/sckott/oai
BugReports: https://github.com/sckott/oai/issues
Imports:
    methods,
    stats,
    utils,
    xml2,
    httr
Suggests:
    testthat
    1. URL for the package (the development repository, not a stylized html page)

https://github.com/sckott/oai

    1. What data source(s) does it work with (if applicable)?

Any OAI-PMH service

    1. Who is the target audience?

Primarily other R clients - I have a number of R clients I work on that could use this. In addition, some users may want to use this to interact with OAI-PMH services themselves.

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

There is an OAI-PMH client (https://cran.rstudio.com/web/packages/OAIHarvester/) on CRAN but it's built on XML and RCurl, packages basically replaced now by xml2 and httr/curl, respectively. oai is built on xml2 and httr. In addition, I give back tidy dplyr like data.frame's to make data comprehension, manipulation, and visualization easier, whereas OAIHarvester gives back arrays/matrices

    1. Check the box next to each policy below, confirming that you agree. These are mandatory.
  • This package does not violate the Terms of Service of any service it interacts with.
  • The repository has continuous integration with Travis and/or another service
  • The package contains a vignette
  • The package contains a reasonably complete readme with devtools install instructions
  • The package contains unit tests
  • The package only exports functions to the NAMESPACE that are intended for end users
    1. Do you agree to follow the rOpenSci packaging guidelines? These aren't mandatory, but we strongly suggest you follow them. If you disagree with anything, please explain.
  • Are there any package dependencies not on CRAN?
  • Do you intend for this package to go on CRAN?
  • Does the package have a CRAN accepted license?
  • Did devtools::check() produce any errors or warnings? If so paste them below.
    1. Please add explanations below for any exceptions to the above:
    1. If this is a resubmission following rejection, please explain the change in circumstances.
@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Aug 25, 2015

Member

@karthik can do, thanks!

Member

sckott commented Aug 25, 2015

@karthik can do, thanks!

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Oct 7, 2015

Member

@karthik should I ask someone else

Member

sckott commented Oct 7, 2015

@karthik should I ask someone else

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Oct 19, 2015

Member

closing, i'll submit again later

Member

sckott commented Oct 19, 2015

closing, i'll submit again later

@sckott sckott closed this Oct 19, 2015

@karthik karthik reopened this Oct 19, 2015

@karthik

This comment has been minimized.

Show comment
Hide comment
@karthik

karthik Oct 19, 2015

Member

Review of OAI

Major issues
The package works as intended and I did not find any major issues with the intended functionality.

Minor issues:

  • The acronym (OAI) is never expanded even once. It would be nice for the DESCRIPTION file to say "Open Archives Initiative Protocol for Metadata Harvesting". It is mentioned in the README which I appreciate.
  • List suggests in alphabetical order recommended here.
  • On package load:
The following objects are masked from 'package:dplyr':

    id, type_sum

Could these benefit from less conflicting names? I know it's already on CRAN now which makes it difficult to change, but avoiding conflicts with popular packages like dplyr is generally helpful.

  • trunc_mat and other functions in tbl_df.R could use at least a one line commented description even if they are internal functions.
  • Is there a reason rbind_fill is copied over from plyr into the package besides trying to minimize one more dependency? I would suggest not doing that in figure since it doesn't seem like good practice to duplicate functions that way.

tests

All tests pass for me locally under R 3.2.2 and R devel.

with apologies for the slow review. 😞

Member

karthik commented Oct 19, 2015

Review of OAI

Major issues
The package works as intended and I did not find any major issues with the intended functionality.

Minor issues:

  • The acronym (OAI) is never expanded even once. It would be nice for the DESCRIPTION file to say "Open Archives Initiative Protocol for Metadata Harvesting". It is mentioned in the README which I appreciate.
  • List suggests in alphabetical order recommended here.
  • On package load:
The following objects are masked from 'package:dplyr':

    id, type_sum

Could these benefit from less conflicting names? I know it's already on CRAN now which makes it difficult to change, but avoiding conflicts with popular packages like dplyr is generally helpful.

  • trunc_mat and other functions in tbl_df.R could use at least a one line commented description even if they are internal functions.
  • Is there a reason rbind_fill is copied over from plyr into the package besides trying to minimize one more dependency? I would suggest not doing that in figure since it doesn't seem like good practice to duplicate functions that way.

tests

All tests pass for me locally under R 3.2.2 and R devel.

with apologies for the slow review. 😞

@karthik

This comment has been minimized.

Show comment
Hide comment
@karthik

karthik Oct 19, 2015

Member

I forgot to add. No objections to moving this into the ropensci org account.

Member

karthik commented Oct 19, 2015

I forgot to add. No objections to moving this into the ropensci org account.

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Oct 21, 2015

Member

The acronym (OAI) is never expanded even once. It would be nice for the DESCRIPTION file to say "Open Archives Initiative Protocol for Metadata Harvesting". It is mentioned in the README which I appreciate.

will do

List suggests in alphabetical order recommended here.

Will do

On package load: Could these benefit from less conflicting names? I know it's already on CRAN now which makes it difficult to change, but avoiding conflicts with popular packages like dplyr is generally helpful.

yeah, can change names

trunc_mat and other functions in tbl_df.R could use at least a one line commented description even if they are internal functions.

will do

Is there a reason rbind_fill is copied over from plyr into the package besides trying to minimize one more dependency? I would suggest not doing that in figure since it doesn't seem like good practice to duplicate functions that way.

To reduce dependencies. But I'll just import plyr

Member

sckott commented Oct 21, 2015

The acronym (OAI) is never expanded even once. It would be nice for the DESCRIPTION file to say "Open Archives Initiative Protocol for Metadata Harvesting". It is mentioned in the README which I appreciate.

will do

List suggests in alphabetical order recommended here.

Will do

On package load: Could these benefit from less conflicting names? I know it's already on CRAN now which makes it difficult to change, but avoiding conflicts with popular packages like dplyr is generally helpful.

yeah, can change names

trunc_mat and other functions in tbl_df.R could use at least a one line commented description even if they are internal functions.

will do

Is there a reason rbind_fill is copied over from plyr into the package besides trying to minimize one more dependency? I would suggest not doing that in figure since it doesn't seem like good practice to duplicate functions that way.

To reduce dependencies. But I'll just import plyr

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Oct 21, 2015

Member

all changes made

Member

sckott commented Oct 21, 2015

all changes made

@sckott

This comment has been minimized.

Show comment
Hide comment
@sckott

sckott Oct 21, 2015

Member

closing - and moving into ropensci now

Member

sckott commented Oct 21, 2015

closing - and moving into ropensci now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment