getlandsat package

[sckott]

Summary

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

getlandsat provides access to Landsat (https://landsat.usgs.gov/) 8 metadata and images hosted on AWS S3 at https://aws.amazon.com/public-data-sets/landsat/ The package only fetches data for users, and does not aid in downstream usage, but additional functionality may be added if deemed necessary.

• Paste the full DESCRIPTION file inside a code block below:

Package: getlandsat
Type: Package
Title: Get Landsat 8 Data
Description: Get Landsat 8 Data from AWS public data sets. Includes
    functions for listing images and fetching them.
Version: 0.0.6.9000
Date: 2016-07-13
Authors@R: c(person("Scott", "Chamberlain", role = c("aut", "cre"),
    email = "myrmecocystus@gmail.com"))
License: MIT + file LICENSE
URL: https://github.com/ropenscilabs/getlandsat
BugReports: https://github.com/ropenscilabs/getlandsat/issues
Imports:
    methods,
    readr (>= 0.2.2),
    httr (>= 1.0.0),
    xml2,
    data.table,
    tibble,
    rappdirs
Suggests:
    testthat,
    covr
RoxygenNote: 5.0.1

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

https://github.com/ropenscilabs/getlandsat

• Who is the target audience?

Anyone that wants to use Landsat images - ecologists, map makers, etc.

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

None that I know of

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 with Travis CI and/or another service.

Publication options

• Do you intend for this package to go on CRAN? No
• 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.
  • The package is deposited in a long-term repository with the DOI:

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:
• If this is a resubmission following rejection, please explain the change in circumstances:

[noamross]

Editor checks:

• Fit: The package meets or criteria 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

Two things from initial check, not enough to hold up review:

• One NOTE: Non-standard file/directory found at top level: ‘Makefile’
• Code coverage appears only partially set up: there's a badge in the repository but I think the appropriate line is not .travis.yml to run covr()

---

Reviewers: @andeek
Due date: 2016-08-05

[andeek]

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 (such as being a major contributor to the software).

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

Estimated hours spent reviewing: 4.5

---

Review Comments

This package is very well done. I especially liked the caching functionality as the images can be somewhat large and slow to download. The functionality seems very self explanatory except for one thing. I am unsure of when to use lsat_list() vs. lsat_scenes(). Perhaps a distinction could be added to the vignette. I have included below individual comments based on each portion of the reviewer template.

Documentation

A statement of need

This is partially true, there is a clear statement in the README of what the package does, but not who the target audience is. Perhaps the section from the summary in the review issue could be added to the README as a more thorough explanation of the package goals and audience.

Installation instructions:

Installation instructions in the README worked perfectly and were very clear.

Vignette(s)

The vignette ran successfully and does demonstrate the major functionality in the paper. It might be helpful to talk a little bit in the vignette about what the list_scenes and list_scene_files functions are doing and what they return in addition to showing their use.

Function Documentation:

All exported functions are well documented in help. Easy to read and understand.

Examples

I ran into a problem with the following example:

  lsat_cache_delete(files = lsat_cache_list()[1])

within the function, there is the following line that duplicates a portion of the path.

  files <- file.path(lsat_path(), "L8", dat$wrs_path, dat$wrs_row, 
        dat$str, files)  

dat$wrs_path, dat$wrs_row, dat$str are all included in the path from lsat_cache_list()[1] so that when I run the code I receive the following error:

  Error: These files don't exist or can't be found: 
     /Users/andeek/Library/Caches/landsat-pds/L8/001/003/LC80010032014272LGN00/L8/001/003/LC80010032014272LGN00/LC80010032014272LGN00_B1.TIF.ovr

Other than that, all examples work as expected.

Community guidelines

I found a contributor code of conduct, but not a guide for how to contribute. The DESCRIPTION file is complete with URL, Maintainer and BugReports.

Functionality

Installation:

No problems with installation.

Functionality:

See the comment on examples for issue with lsat_cache_delete. Other than that small problem, the software functions great!
Automated tests:

There were no unit tests for the caching functions, but I'm not sure if that's because it's specific to the user's machine so would be harder to write? All the other functions are tested extensively and pass on my machine.

Packaging guidelines

• Package naming - great name!
• Function/variable naming - all functions are names with snake case and avoid name conflicts.
• README - has almost everything required. The only thing I don't see is the rOpenSci footer image.
• Code of conduct - included
• Documentation - all the functions are documented very well and there is a vignette as well as top-level documentation.
• News - this is missing
• Authorship - looks good
• Package dependencies - looks good
• Testing - testthat is being used, see my above comment on testing the cache functions.
• Continuous integration - using Travis CI
• Console messages - cat is only used in print.*() methods and otherwaise warning and message are used.
• Recommended software scaffolding - using httr
• Miscellaneous CRAN gotchas - these all look good

[sckott]

Thanks very much @andeek - I'll get to this asap

[sckott]

One NOTE: Non-standard file/directory found at top level: ‘Makefile’

will rbuildignore that ropensci-archive/getlandsat#13

Code coverage appears only partially set up: there's a badge in the repository but I think the appropriate line is not .travis.yml to run covr()

i think i stopped doing it cause it was failing for some weird reason, will get it back started

---

This package is very well done. I especially liked the caching functionality as the images can be somewhat large and slow to download.

thanks!

The functionality seems very self explanatory except for one thing. I am unsure of when to use lsat_list() vs. lsat_scenes() Perhaps a distinction could be added to the vignette.

Thanks, i'll clarify ropensci-archive/getlandsat#14

there is a clear statement in the README of what the package does, but not who the target audience is. Perhaps the section from the summary in the review issue could be added to the README as a more thorough explanation of the package goals and audience.

thanks, will do ropensci-archive/getlandsat#15

vignette .... It might be helpful to talk a little bit in the vignette about what the list_scenes and list_scene_files functions are doing and what they return in addition to showing their use.

okay, will do ropensci-archive/getlandsat#16

I ran into a problem with the following example: lsat_cache_delete(files = lsat_cache_list()[1])

I'll fix that and write a test for it, thanks ropensci-archive/getlandsat#17

I found a contributor code of conduct, but not a guide for how to contribute.

will add a CONTRIBUTING.md file ropensci-archive/getlandsat#18

There were no unit tests for the caching functions ...

i think I avoided that as they would take a while to run, but i'll try to add some ropensci-archive/getlandsat#19

README - has almost everything required. The only thing I don't see is the rOpenSci footer image.

thanks for noting, will add

News - this is missing

thanks for noting, will add

[sckott]

@andeek everything has been addressed now

@noamross anything else, or good to go?

[sckott]

@noamross anything else to do?

[andeek]

Everything above has been addressed and I've edited the comment to reflect that. Thanks @sckott and @noamross!

[sckott]

thanks again @andeek for the review

@noamross Date removed ropensci-archive/getlandsat@0252fb3

[noamross]

Final checks all pass. Approved! @sckott you know the drill from here.