IQSS / dataverse-client-r

R Client for Dataverse Repositories
https://iqss.github.io/dataverse-client-r
61 stars 25 forks source link

* Missing json decoding of response for dataset_versions #28

Closed billy34 closed 3 years ago

billy34 commented 4 years ago

Should solve issue #27

wibeasley commented 4 years ago

@billy34, thanks for noticing this in #27 and filing this PR. Because I'm new to this package, I'm reluctant to accept any PRs without an accompanying test. And unfortunately we haven't developed a test suite that's easy for others to add a few tests with a PR of a new feature/correction like yours.

I'd like to add at least a minimal test suite that incorporates this, but realistically that might be a few weeks (see #29).

@pdurbin, @leeper, or anyone else, I don't want to delay @billy34's contribution unnecessarily. Please accept the PR if you'd like.

pdurbin commented 4 years ago

@wibeasley I'm fully supportive of #29 and the issues you linked there: #4 and #22

You are completely sane to want tests in place before merging pull requests. :smile:

@billy34 thanks for the pull request! As the brand new maintainer of dataverse-client-r (thanks again!) @wibeasley has inherited a ton of code so please be patient as he comes up to speed and gets comfortable. :smile: If you have any thoughts on adding tests, please let us know!

wibeasley commented 4 years ago

@billy34 , I haven't forgotten about this PR.

Will you please

  1. add yourself to the DESCRIPTION file as a "ctb". Add your ORCID id if you have one.
  2. update NEWS.md to reflect this improvement?

I'm using R Packages for the definition/distinction between the "aut" and "ctb".

wibeasley commented 4 years ago

@billy34, thanks for this. I see that the Travis error are coming from some breaking change in the covr prereq, and nothing to do with your code (or even the package's code).

kuriwaki commented 3 years ago

@billy34: I fixed unmatched parenthesis in the author listing that were preventing the package from compiling. Anyhow, after that the package_versions don't work. Maybe it's due to changes introduced since your PR. Do these two reprex examples below work for you?

library("dataverse")

## set server internally
Sys.setenv("DATAVERSE_SERVER" = "dataverse.harvard.edu")

# 1. from help page of dataset_versions
monogan <- get_dataverse("monogan")
monogan_data <- dataverse_contents(monogan)
d1 <- get_dataset(monogan_data[[1]])
class(d1)
#> [1] "dataverse_dataset"
dataset_versions(d1)
#> Error in dataset_versions(d1): Not Found (HTTP 404).

# 2. from another simple example on DEMO
d2 <- get_dataset(dataset = "doi:10.70122/FK2/PPIAXE",
                  server = "demo.dataverse.org")
class(d2)
#> [1] "dataverse_dataset"
dataset_versions(d2, server = "demo.dataverse.org")
#> Error in dataset_versions(d2, server = "demo.dataverse.org"): Not Found (HTTP 404).

Created on 2020-12-30 by the reprex package (v0.3.0)

kuriwaki commented 3 years ago

I don't know JSON as well but it seems 0dc86ca has the same effect as the proposed JSON edit here and is simpler.