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

nasapower #155

Closed
15 tasks done
adamhsparks opened this issue Nov 4, 2017 · 74 comments
Closed
15 tasks done

nasapower #155

adamhsparks opened this issue Nov 4, 2017 · 74 comments

Comments

@adamhsparks
Copy link
Member

adamhsparks commented Nov 4, 2017

Summary

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

nasapower automates downloading NASA-POWER agroclimatology data in your R
session as a tidy data frame for analysis, modelling or other purposes by interfacing with the NASA POWER API. POWER (Prediction Of Worldwide Energy Resource) data are freely available for download through a web interface at a resolution of 0.5˚ longitude by 0.5˚ latitude.

  • Paste the full DESCRIPTION file inside a code block below:
Package: nasapower
Type: Package
Title: NASA-POWER Data from R
Version: 1.0.0.9000
Authors@R: person(given = "Adam H.", family = "Sparks",
    role = c("aut", "cre"), email = "adamhsparks@gmail.com",
    comment = c(ORCID = "0000-0002-0061-8359"))
URL: https://github.com/adamhsparks/nasapower
BugReports: https://github.com/adamhsparks/nasapower/issues
Description: Download NASA-POWER global meteorology and surface solar energy
    climatology data and create a tidy data frame for 141 parameters.  nasapower
    provides an interface to the NASA - POWER API.  POWER (Prediction Of
    Worldwide Energy Resource) data are freely available for download with a
     resolution of 0.5 by 0.5 arc degree longitude and latitude and are funded
    through the NASA Earth Science Directorate Applied Science Program. For more
    on the data themselves, please see <https://power.larc.nasa.gov/>.
Depends:
    R (>= 3.2.0)
License: MIT + file LICENSE
Imports:
    crul,
    lubridate,
    jsonlite,
    readr,
    tibble
RoxygenNote: 6.0.1
Roxygen: list(markdown = TRUE)
Encoding: UTF-8
NeedsCompilation: no
Repository: CRAN
LazyData: TRUE
ByteCompile: TRUE
Suggests:
    APSIM,
    covr,
    dplyr,
    ggplot2,
    knitr,
    raster,
    rmarkdown,
    testthat,
    vcr
VignetteBuilder: knitr
X-schema.org-applicationCategory: Tools
X-schema.org-keywords: NASA, meteorological-data, weather, global, weather, weather-data, meteorology, NASA-POWER, agroclimatology, earth-science, data-access
  • URL for the package (the development repository, not a stylized html page):

https://github.com/adamhsparks/nasapower

  • Please indicate which category or categories from our package fit policies this package falls under *and why(? (e.g., data retrieval, reproducibility. If you are unsure, we suggest you make a pre-submission inquiry.):

data retrieval and extraction, nasapower automates downloading and parsing
NASA - POWER data into a useful format in R using the POWER API.

  • Who is the target audience?

Researchers who use NASA - POWER for modelling or other purposes though there may
be others that I'm not familiar with. This is the audience I know best since
I'm part of it and use these data for my own work.

Since the POWER API is brand new, no. This is the first R package to utilise it's capabilities.

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?
  • Do you wish to automatically submit to the Journal of Open Source Software? If so:
    • The package has an obvious research application according to JOSS's definition.
    • The package contains a paper.md matching JOSS's requirements 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:

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

I've picked on everyone I can think of previously for other submissions.

@sckott sckott added the package label Nov 4, 2017
@adamhsparks
Copy link
Member Author

adamhsparks commented Nov 4, 2017

I was looking at the POWER website. The most, but not all, of the other POWER datasets have similar interfaces. It should be pretty simple add some of the Renewable Energy and Sustainable Buildings data to the package as well. I've just never used them for my work. It would be good if these were added, that someone with some knowledge of them could provide some assistance on what's useful.

Happy to get feedback on that.

@sckott
Copy link
Contributor

sckott commented Nov 5, 2017

Thanks @adamhsparks - Editors are discussing ...

@sckott
Copy link
Contributor

sckott commented Nov 7, 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

Thanks for your submission @adamhsparks

Goodpractice output:

── 0 errors| 0 warnings| 0 notes ✔
♥ Aha! Astounding package! Keep up the best work!

Seeking reviewers now.


Reviewers: @alisonboyer @UniversalTourist
Due date: 2017-12-06

@sckott
Copy link
Contributor

sckott commented Nov 9, 2017

Reviewers are @alisonboyer and @UniversalTourist - due date is 2017-12-06

@UniversalTourist
Copy link

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).
For packages co-submitting to JOSS

The package contains a paper.md matching JOSS's requirements 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).

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: It took almost, totally, 4-5 hours to check everything. During that time I studied what is NASA-POWER, agroclimatology and read rOpenSci guidelines, learned about Travis-CI and running tests.


Review Comments

This is the first time I heard about NASA-POWER and it is a great data source for Earth scientists. These package provides a data frame output and it will make the data preparation process more easier than copy-pasting.
(I have tested functions for region of my country and already sent the package's Github link for my colleges :) )

  • Installation:
    With MacOS:
    I used macOS Sierra, Verison 10.12 and I was able to install the package from Github without any problem by devtools.

  • Code Overall:
    These package has two functions to get data for a cell or for a region. Both functions has nice structered, easy to follow and also nice explained with comments, I enjoyed reviewing this part especially. (I also learned about how httr works by running these codes step by step.)

  • rOpenSci criteria:
    Package name is really nice. Function and variable naming is proper. Licence and Code of Conduct are available.

  • Readme:
    Readme.rmd is clear and easy to follow steps for installations.

  • Vignette:
    There are more details and examples about other function. Also the examples provide more information and help to illustrate the importance of the package.
    In the introduction part there is only one mistake, the "which will" words are written for two times: "both of which will which will download".
    I notified author about plots on vignette which seems different when I run on my system, and problem was resolved with a commit fastly when I contacted with the author.

  • Errors and console messages:
    They are all clear.

  • R CMD check:
    R CMD check passes without any warnings or any errors.

  • 0 errors | 0 warnings | 0 notes
    :)

  • Other comments
    NASA-POWER provides data at a resolution of 1˚ longitude by 1˚ latitude, which is good enough most of the time. Maybe you can also write about other data sources which provide higher resolutions in the package readme file for the people who come to this package repo for higher resolution.

p.s This is my first time at reviewing a package and I hope I can be helpful :)

@sckott
Copy link
Contributor

sckott commented Nov 27, 2017

thx very much for your review @UniversalTourist

@adamhsparks
Copy link
Member Author

adamhsparks commented Nov 27, 2017

Thank you @UniversalTourist

I have fixed the vignette text you noted where it said "which will which will", ropensci/nasapower@6dd0d9c.

I have also added a link to other gridded climatology data sets. As far as I know, nothing really aligns with what NASA POWER offers at a higher resolution, but there are other gridded climatology data sets out there, ropensci/nasapower@8b8a11a

@UniversalTourist
Copy link

Thank you very much!

@sckott
Copy link
Contributor

sckott commented Dec 8, 2017

@alisonboyer Your review was due a few days ago. Can you get it in soon?

@alisonboyer
Copy link

alisonboyer commented Dec 12, 2017 via email

@sckott
Copy link
Contributor

sckott commented Dec 12, 2017

thanks @alisonboyer

@alisonboyer
Copy link

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

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
    Comment: How large of a region can you request using this service? Some notes about the expected size of the data frame returned would be helpful. For example, a 2 x 4 degree region with 3 variables and all available time steps was ~ 2MB. If I request the entire globe, what will happen?
  • 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.
    Comment: I do not know how to run unit tests. My local tests of the basic functionality all ran flawlessly.
  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines

Estimated hours spent reviewing: 2.5

Review Comments:

Providing a recommended citation for the data is very nice. Does the data have a DOI?
The link to other climate data provided by NOAA ESRL is helpful.

@adamhsparks
Copy link
Member Author

adamhsparks commented Dec 13, 2017

Thank you, @alisonboyer.

I didn't have a statement about performance because it will vary by the data requested and Internet connection. However, I ran an example this morning and added a statement to the vignette in this commit, ropensci/nasapower@3bab17e about what some possible realistic times and data frame sizes could be for this. See starting at line 114 in the nasapower.Rmd vignette for the statement.

AFAIK there is no DOI for the POWER data. As you noted, I've included their statement about how they wish to be acknowledged and the link to https://power.larc.nasa.gov/documents/Agroclimatology_Methodology.pdf is included in the documentation in several locations as well.

@adamhsparks
Copy link
Member Author

adamhsparks commented Dec 13, 2017

For both @UniversalTourist and @alisonboyer, I've also expanded the documentation in the vignette to provide an example of how to turn the data frame in to a raster::brick spatial object for users that need it to be a spatial object not a normal data frame.

https://adamhsparks.github.io/nasapower/articles/nasapower.html#creating-spatial-objects

@adamhsparks
Copy link
Member Author

adamhsparks commented Dec 13, 2017

This afternoon the POWER webpage stopped serving data. I've added a check now that pings the server first, five times, to see if it's responding, before proceeding with the request.

If the reviewers could check this as it was added after both reviews?

ropensci/nasapower@94e2440

@UniversalTourist
Copy link

UniversalTourist commented Dec 13, 2017

Hey Adam!
I will check bot spatial example and this ping issue later today! And let you know.

Cheers.

@adamhsparks
Copy link
Member Author

@UniversalTourist, please wait. Now that it's responding again, it's clear that my fix didn't work. I need to find a new way.

@adamhsparks
Copy link
Member Author

adamhsparks commented Dec 13, 2017

OK, fixed now with a new better way of doing checking whether the NASA - POWER site is serving data or not.

Sorry about that.

@sckott
Copy link
Contributor

sckott commented Aug 21, 2018

@alisonboyer can you get another review in within a week ? if not no worries and we'll move on

@alisonboyer
Copy link

alisonboyer commented Aug 22, 2018 via email

@sckott
Copy link
Contributor

sckott commented Sep 19, 2018

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

Good idea to add guidelines for users on how to contribute. See e.g. https://github.com/ropensci/dotgithubfiles/

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

Review Comments

awesomeness:

  • nice work!
  • nice failure behavior!

other comments:

  • duplicate data in REAMDE surface solar energy climatology data data
  • you could be a bit more detailed on your parameters man file, e.g., A list with 141 objects. What kind of objects? And it looks like the list is named by something useful?
  • Having examples for all functions is good, but the examples are pretty sparse, only 1 for each fxn.
  • including @return for each fxn would help users know what to expect
  • nice use of parameter checking in your fxns. for logical parameters, there doesn't seem to be checks. so e.g., a user could do get_power(meta = 5), though unlikely could happen i guess
  • it's probably a good idea for examples and tests to write to tmp dir instead of a non-temporary user path, e.g., dsn = "~/Documents"
  • this failure behavior isn't great when dsn and or file_out not given (same for create_met):
create_icasa(lonlat = c(151.81, -27.48),
             dates = c("1985-01-01", "1985-12-31")
)
Error in if (substr(file_out, nchar(file_out) - 3, nchar(file_out)) !=  : 
  argument is of length zero

@adamhsparks
Copy link
Member Author

Thanks @sckott, I'll address your comments shortly.

@adamhsparks
Copy link
Member Author

@sckott, did you have any thoughts on the fact that get_power() returns a list or a data frame depending the meta flag? I can't think of a better way to handle it aside from inserting the meta-information into the data frame along with the data, is that a good idea or necessary or is how I've handled it the best way to, what do you think?

@sckott
Copy link
Contributor

sckott commented Sep 19, 2018

I'm sure I have some fxns that return different objects depending on user inputs, but it definitely seems better to avoid that. Maybe one of these two optoins:

  • put the metadata in attributes in the data.frame, then user can access them with attributes or some custom thing
  • make a custom print fxn where the metadata is printed on top of the data.frame sort of like tibbles do, then the object itself is still a data.frame

@adamhsparks
Copy link
Member Author

adamhsparks commented Sep 19, 2018

I've some ideas of what can be done with it. I'll think on it some more and see what I can implement.

At this point I think I've addressed everything else you've pointed out above.

@sckott
Copy link
Contributor

sckott commented Sep 20, 2018

I've some ideas of what can be done with it. I'll think on it some more and see what I can implement.

should I wait on this for this submission, or do you mean more in the long term?

@adamhsparks
Copy link
Member Author

No, please wait, I've pushed a custom print() method last night that includes an attribute header to the devel branch. Polishing it, will be done within 48 hrs I'd guess?

@sckott
Copy link
Contributor

sckott commented Sep 20, 2018

will do.

@adamhsparks
Copy link
Member Author

Hi @sckott, I think it's ready for you to check and hopefully accept.

I've found and squashed several more bugs while dealing with the metadata and your suggestions.

cheers!

@sckott
Copy link
Contributor

sckott commented Sep 21, 2018

@adamhsparks a few things:

  • the example for create_icasa has file = ICASA_example.txt where the file name is not quoted, R is looking for an object named ICASA_example.txt
  • for the create_met example you have dsn = tmpdir(), which sholuld be tempdir() -
  • the get_power 2nd example doesn't work either. you have temporal_average as a required parameter, but don't supply anything to it in the 2nd example.
  • Also about get_power. it's not a good idea to have a required parameter (w/o a default) after a non-required parameter. that is, you have function(community, lonlat, pars, dates = NULL, temporal_average), whereas function(community, lonlat, pars, temporal_average, dates = NULL) would make more sense b/c all required paramters are first, then the last is optional

Its essential that your examples run successfully. a good way to do this is have them run on travis, e.g. https://github.com/ropensci/wellknown/blob/master/.travis.yml#L28 Please make sure your examples work moving forward

@adamhsparks
Copy link
Member Author

adamhsparks commented Sep 21, 2018

Thanks @sckott, I'll address these issues.

I've not seen that Travis configuration before, that's useful!

@adamhsparks
Copy link
Member Author

Hi @sckott, after that embarrassing blunder, I've corrected the examples and a few other issues to align with rOpenSci guidelines if you'd like to have a look.

Thanks again for the suggestion with Travis to build examples, it caught yet another error for me even after I'd fixed everything once!

@sckott
Copy link
Contributor

sckott commented Sep 24, 2018

nice, glad it was helpful.

having another quick look

@sckott
Copy link
Contributor

sckott commented Sep 24, 2018

Approved! Thanks again for your submission @adamhsparks !

  • Please transfer the package to ropensci- You'll be made admin once you do.
  • Make sure to
    • add rOpenSci footer to README
      [![ropensci_footer](https://ropensci.org/public_images/ropensci_footer.png)](https://ropensci.org)
    • 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

@stefaniebutland
Copy link
Member

Hi @adamhsparks ! You know the blog post drill from https://ropensci.org/blog/2017/04/04/gsodr/ 😄. Are you interested in doing one, or a tech note on nasapower?

Tech Notes can be published at any time, while we publish blog posts about packages weekly. Right now Tuesday 2018-10-16 or subsequent Tuesdays are available for blog posts. If you're interested, let me know your preferred publication date and then please submit via pull request a week prior. Guidelines are here: https://github.com/ropensci/roweb2#contributing-a-blog-post

@adamhsparks
Copy link
Member Author

Excellent! I'm tidying up a few remaining issues and will transfer it over shortly.

@sckott and @stefaniebutland, yes, we should do a proper blog post about this one. I'll get something together and submit a PR next month.

Thank you all, once again, for the support and motivation. This package has been a long-time coming, but I'm super happy with the condition it's now in before it even reaches CRAN (which will hopefully happy soon as well).

@adamhsparks
Copy link
Member Author

Ok, I've transferred ownership and once again lost the ability to edit the webpage and the tags associated with the repository so the webpage link incorrectly points to my personal repository, still, as I forgot to change that before transferring it over.

Can I get the right permissions to edit the repository details, etc.?

ta!

@sckott
Copy link
Contributor

sckott commented Sep 25, 2018

nice work adam!

@sckott
Copy link
Contributor

sckott commented Sep 25, 2018

done

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