rsi: Efficiently Retrieve and Process Satellite Imagery #636
Comments
|
Thanks for submitting to rOpenSci, our editors and @ropensci-review-bot will reply soon. Type |
|
🚀 Editor check started 👋 |
|
Another quick note -- I will not be able to transfer this repository to rOpenSci if accepted. I had asked on Slack and was told by Yani that this was acceptable, though it's only mentioned in the book here. I want to flag this at the start, in case it turns out to be an issue! |
Checks for rsi (v0.2.0.9000)git hash: 694d2a5f
Important: All failing checks above must be addressed prior to proceeding (Checks marked with 👀 may be optionally addressed.) Package License: Apache License (>= 2) 1. Package DependenciesDetails of Package Dependency Usage (click to open)
The table below tallies all function calls to all packages ('ncalls'), both internal (r-base + recommended, along with the package itself), and external (imported and suggested packages). 'NA' values indicate packages to which no identified calls to R functions could be found. Note that these results are generated by an automated code-tagging system which may not be entirely accurate.
Click below for tallies of functions used in each package. Locations of each call within this package may be generated locally by running 's <- pkgstats::pkgstats(<path/to/repo>)', and examining the 'external_calls' table. basenames (20), c (19), class (10), lapply (9), length (8), vapply (8), args (6), formals (6), list (6), mget (6), tryCatch (5), file.path (4), for (4), max (4), min (4), options (4), tempfile (4), ifelse (3), url (3), all (2), call (2), character (2), drop (2), eval (2), is.null (2), nrow (2), paste (2), paste0 (2), unlist (2), with (2), col (1), data.frame (1), dirname (1), get (1), grep (1), mapply (1), merge (1), ncol (1), numeric (1), readLines (1), replicate (1), seq_len (1), source (1), str2lang (1), suppressWarnings (1), t (1), tempdir (1), tolower (1), toupper (1), vector (1) rsibuild_progressr (5), spectral_indices (3), extract_urls (2), remap_band_names (2), alos_palsar_mask_function (1), calc_scale_strings (1), calculate_indices (1), check_indices (1), check_type_and_length (1), default_query_function (1), download_web_indices (1), filter_bands (1), filter_platforms (1), get_alos_palsar_imagery (1), get_dem (1), get_landsat_imagery (1), get_naip_imagery (1), get_rescaling_formula (1), get_sentinel1_imagery (1), get_sentinel2_imagery (1), get_stac_data (1), is_pc (1), landsat_mask_function (1), maybe_sign_items (1), set_gdalwarp_extent (1), spectral_indices_url (1) rlangarg_match (4), caller_env (2), warn (2), exec (1), new_environment (1) terrarast (5), sprc (2), crs (1), nlyr (1), predict (1) rstacassets_url (2), items_datetime (2), stac_search (1) glueglue (4) methodsis (3) future.applyfuture_lapply (2) statspredict (1), setNames (1) toolsfile_ext (1), R_user_dir (1) httruser_agent (1) progressrprogressor (1) sfst_bbox (1) NOTE: Some imported packages appear to have no associated function calls; please ensure with author that these 'Imports' are listed appropriately. 2. Statistical PropertiesThis package features some noteworthy statistical properties which may need to be clarified by a handling editor prior to progressing. Details of statistical properties (click to open)
The package has:
Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the The final measure (
2a. Network visualisationClick to see the interactive network visualisation of calls between objects in package 3.
|
| id | name | conclusion | sha | run_number | date |
|---|---|---|---|---|---|
| 8486854009 | Lock Threads | success | 694d2a | 156 | 2024-03-30 |
| 8482543797 | pages build and deployment | success | 1de384 | 65 | 2024-03-29 |
| 8482287996 | pkgdown | success | 694d2a | 135 | 2024-03-29 |
| 8482287992 | R-CMD-check | success | 694d2a | 131 | 2024-03-29 |
| 8482287990 | R-CMD-check-hard | success | 694d2a | 131 | 2024-03-29 |
| 8482287998 | test-coverage | success | 694d2a | 131 | 2024-03-29 |
3b. goodpractice results
R CMD check with rcmdcheck
rcmdcheck found no errors, warnings, or notes
Test coverage with covr
ERROR: Test Coverage Failed
Cyclocomplexity with cyclocomp
The following functions have cyclocomplexity >= 15:
| function | cyclocomplexity |
|---|---|
| get_stac_data | 44 |
| stack_rasters | 28 |
| check_type_and_length | 25 |
Static code analyses with lintr
lintr found the following 127 potential issues:
| message | number of times |
|---|---|
| Avoid library() and require() calls in packages | 4 |
| Lines should not be more than 80 characters. | 123 |
4. Other Checks
Details of other checks (click to open)
✖️ The following 2 function names are duplicated in other packages:
-
calculate_indicesfrom ClusterStability
-
sign_planetary_computerfrom rstac
Package Versions
| package | version |
|---|---|
| pkgstats | 0.1.3.11 |
| pkgcheck | 0.1.2.21 |
Editor-in-Chief Instructions:
Processing may not proceed until the items marked with ✖️ have been resolved.
|
Guessing covr fails due to not setting my custom This environment variable is used to skip a test on my covr CI. The tl;dr is that rsi executes some code in a minimal environment to protect against malicious code downloaded from the internet, which prevents covr from injecting its tracking inside of that minimal environment. I still want the file to be tested (and the other pieces of the file to be counted in coverage), though, so I wrapped the local environment section in You can see my code coverage report at https://app.codecov.io/gh/Permian-Global-Research/rsi and my CI workflow for this at https://github.com/Permian-Global-Research/rsi/blob/main/.github/workflows/test-coverage.yaml |
|
@ropensci-review-bot assign @jhollist as editor |
|
Assigned! @jhollist is now the editor |
|
@mikemahoney218 Been swamped these last few days. I will work on digging through this today and tomorrow and get back to you soon and should hopefully be ready to start finding reviewers. I am looking forward to this review. Does look like an interesting package! |
|
No worries, and thanks for the update! |
|
@ropensci-review-bot check rsi |
|
I'm sorry human, I don't understand that. You can see what commands I support by typing:
|
|
@ropensci-review-bot check package |
|
Thanks, about to send the query. |
|
🚀 Editor check started 👋 |
|
Just want to flag that I'm still expecting (your version of) covr to fail, due to #636 (comment) The core issue is that As a result, I toggle the tests that hit this code path using a custom I don't want to disable the whole .R file from covr, because covr can instrument the rest of the file, and I don't want to drop these tests (or make them off by default) because I'd like |
Checks for rsi (v0.2.0.9000)git hash: e71186f2
Important: All failing checks above must be addressed prior to proceeding (Checks marked with 👀 may be optionally addressed.) Package License: Apache License (>= 2) 1. Package DependenciesDetails of Package Dependency Usage (click to open)
The table below tallies all function calls to all packages ('ncalls'), both internal (r-base + recommended, along with the package itself), and external (imported and suggested packages). 'NA' values indicate packages to which no identified calls to R functions could be found. Note that these results are generated by an automated code-tagging system which may not be entirely accurate.
Click below for tallies of functions used in each package. Locations of each call within this package may be generated locally by running 's <- pkgstats::pkgstats(<path/to/repo>)', and examining the 'external_calls' table. basenames (20), c (19), class (10), lapply (9), length (8), vapply (8), args (6), formals (6), list (6), mget (6), tryCatch (5), file.path (4), for (4), max (4), min (4), options (4), tempfile (4), ifelse (3), url (3), all (2), call (2), character (2), drop (2), eval (2), is.null (2), nrow (2), paste (2), paste0 (2), unlist (2), with (2), col (1), data.frame (1), dirname (1), get (1), grep (1), mapply (1), merge (1), ncol (1), numeric (1), readLines (1), replicate (1), seq_len (1), source (1), str2lang (1), suppressWarnings (1), t (1), tempdir (1), tolower (1), toupper (1), vector (1) rsibuild_progressr (5), spectral_indices (3), extract_urls (2), remap_band_names (2), alos_palsar_mask_function (1), calc_scale_strings (1), calculate_indices (1), check_indices (1), check_type_and_length (1), default_query_function (1), download_web_indices (1), filter_bands (1), filter_platforms (1), get_alos_palsar_imagery (1), get_dem (1), get_landsat_imagery (1), get_naip_imagery (1), get_rescaling_formula (1), get_sentinel1_imagery (1), get_sentinel2_imagery (1), get_stac_data (1), is_pc (1), landsat_mask_function (1), maybe_sign_items (1), set_gdalwarp_extent (1), spectral_indices_url (1) rlangarg_match (4), caller_env (2), warn (2), exec (1), new_environment (1) terrarast (5), sprc (2), crs (1), nlyr (1), predict (1) rstacassets_url (2), items_datetime (2), stac_search (1) glueglue (4) methodsis (3) future.applyfuture_lapply (2) statspredict (1), setNames (1) toolsfile_ext (1), R_user_dir (1) httruser_agent (1) progressrprogressor (1) sfst_bbox (1) NOTE: Some imported packages appear to have no associated function calls; please ensure with author that these 'Imports' are listed appropriately. 2. Statistical PropertiesThis package features some noteworthy statistical properties which may need to be clarified by a handling editor prior to progressing. Details of statistical properties (click to open)
The package has:
Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages
All parameters are explained as tooltips in the locally-rendered HTML version of this report generated by the The final measure (
2a. Network visualisationClick to see the interactive network visualisation of calls between objects in package 3.
|
| id | name | conclusion | sha | run_number | date |
|---|---|---|---|---|---|
| 8572262292 | Commands | skipped | bc9e46 | 46 | 2024-04-05 |
| 8639486278 | Lock Threads | success | e71186 | 168 | 2024-04-11 |
| 8575408691 | pages build and deployment | success | f75519 | 69 | 2024-04-05 |
| 8575285598 | pkgdown | success | e71186 | 140 | 2024-04-05 |
| 8575285595 | R-CMD-check | success | e71186 | 135 | 2024-04-05 |
| 8575285599 | R-CMD-check-hard | success | e71186 | 135 | 2024-04-05 |
| 8575285597 | test-coverage | success | e71186 | 135 | 2024-04-05 |
3b. goodpractice results
R CMD check with rcmdcheck
rcmdcheck found no errors, warnings, or notes
Test coverage with covr
ERROR: Test Coverage Failed
Cyclocomplexity with cyclocomp
The following functions have cyclocomplexity >= 15:
| function | cyclocomplexity |
|---|---|
| get_stac_data | 44 |
| stack_rasters | 28 |
| check_type_and_length | 25 |
Static code analyses with lintr
lintr found the following 127 potential issues:
| message | number of times |
|---|---|
| Avoid library() and require() calls in packages | 4 |
| Lines should not be more than 80 characters. | 123 |
4. Other Checks
Details of other checks (click to open)
✖️ The following 2 function names are duplicated in other packages:
-
calculate_indicesfrom ClusterStability
-
sign_planetary_computerfrom rstac
Package Versions
| package | version |
|---|---|
| pkgstats | 0.1.3.11 |
| pkgcheck | 0.1.2.21 |
Editor-in-Chief Instructions:
Processing may not proceed until the items marked with ✖️ have been resolved.
|
@mikemahoney218 Thanks for the update. And as you expected the bot checks fail. I am not too worried about that given you have good coverage and can demonstrate it with the codecov reports. We will want to make sure that this coverage is reported out in the repositories README. You may already be doing that, I just haven't checked yet! Stay tuned, I am working on this today and tomorrow and expect to move on to finding reviewers shortly after that! |
Editor checks:
Editor commentsI think we are ready to pass on to reviewers! Nice Job! Only one very small request:
I like to see a more upfront CONTRIBUTING. I always get lost trying to find them when embedded inside Also, I have no concerns about the tests failing on the bot. You have implemented them well, the coverage is good, and the badge makes it easy to find. |
|
@ropensci-review-bot seeking reviewers |
|
Please add this badge to the README of your package repository: [](https://github.com/ropensci/software-review/issues/636)Furthermore, if your package does not have a NEWS.md file yet, please create one to capture the changes made during the review process. See https://devguide.ropensci.org/releasing.html#news |
|
Added the link to CONTRIBUTING! |
|
@ropensci-review-bot assign @mdsumner as reviewer |
|
@mdsumner added to the reviewers list. Review due date is 2024-05-09. Thanks @mdsumner for accepting to review! Please refer to our reviewer guide. rOpenSci’s community is our best asset. We aim for reviews to be open, non-adversarial, and focused on improving software quality. Be respectful and kind! See our reviewers guide and code of conduct for more. |
|
@mdsumner just pinging to see if you are finished with the review yet. |
Package ReviewPlease check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide
DocumentationThe package includes all the following forms of documentation:
Functionality
Estimated hours spent reviewing: 7
Review CommentsI really like this package, I see a lot of familiar experiences and the result is a good and consistent set of functions for navigating this space. I haven't personally done anything with spectral indices, usually I expore imagery and how "it looks". I appreciate having all this ease-of-use tooling at hand, and I will be pointing colleagues directly to this package, and I hope to stay involved in at least small ways. Please make mention somewhere that "where the compute runs" (i.e. what "local" means) is important here. There's an issue with performance in different regions - and some pointers to how one might run in a region closer to where the usual data sources are (us-west2 for example. I'm not suggesting that's an easy topic to cover, just would like to see it mentioned. It takes about 2x as long to run examples here in Tasmania, vs computers in us-west (I know that comparison is a lot more complex than this). It might be an idea to point to ways of running computers on public systems that run closer to the data, or at least a guide to that. I'm a bit disappointed in our community in that packages in R have tended towards doing "everything", and (not a criticism of this package) here we are doing a lot of things: getting file sources from STAC (a json web query), configuring for authentication, downloading specific files (a curl task), mosaicing and standardizing imagery (a GDAL task). I like that the top level functions are decomposed in this package itself, and the documentation is clear as to what underlying functions are used, and I can understand why parts of "foundational" packages are used with a mix of underlying generic tools for this complex stack of work. I'm massively impressed with how much this package actually does, and exposes in a general way down to the level of templated types of functionality and details like the GDALWarp options. rsi is a little bit like a datacube, but with exposure of the underlying components at key steps. I really like how the asset is a lightly classed object with other functions that it needs stored as attributes, and that that is how the remaining arguments to the getter function is structured, so it seamlessly inherits each level but also remains flexible to user-changes in that one call. Questions I had that I could put more effort+examples into:I'd like see some validations of the raster values obtained in some examples, if there was an external example that's documented elsewhere (in a python notebook, or another R package) it would be neat to have a clear comparison of the scale/s shown be these raster files obtained here against an independent example. (I had intended to do this myself ...) I was looking for an example where I can make a true colour image with RGB, I don't understand the scaling that occurs in the examples (we have 0-1 scale numbers, which don't work with terra::plotRGB or its stretch argument off the shelf, and leave me unclear about the scaled ranges and whether I am plotting an RGB image correctly. The first example in the readme plots an RGB image as separate bands, and I'm unclear what to do to plot that as an "image". Again, I had meant to provide an actual example here. I believe the advice should be: but also, it's open to interpretation and expert use afaics. Specific notesTwo of these three files have no data in the aoi region. qfiles <- get_landsat_imagery(aoi,
start_date = "2022-06-01", end_date = "2022-06-30",
composite_function = NULL)I tried this naive thing, to run rsi_query_api, and I think at the least it should error fast when not getting a bbox in longlat. it could detect that the bbox input is not in longlat and 1) error or 2) just transform it.
Here I would rather reproject the extent, this is not so hard to do and exists in a few places
On this point, GDAL warp can itself do these things, itself a monolith of abstractions itself but we can possibly avoid downloading entire scenes. I mention this not as a strong suggestion, mainly an invite to discuss further (I'm looking at the rise of {gdalraster} here).
I think this really needs an example, because there's room for guidance on creating a nice tile mosaic definition (with terra::align for example, or actually with st_tile or getTileExtents). It's very important point, and say what if we wanted a (CONUS) state-level imagery. This can be done a little more abstractly using terra::makeTiles and stars::st_tile (which give the index and extents helpfully but separately). (Maybe an assign-to-me task).
I only note this as a discussion-bounce, for possible followup: GDAL can do this too, and has file system abstrations that can copy, info, or warp-in-part to a target grid. In the vignette I like when there's "one change", that's an excellent situation when you can tweak major changes with only one tiny plumbing modification. This example (in the README) should save the file name as an output. It's otherwise a pipeline that I don't get an object for in the end, and it can take some time (in Australia). projected_ashe <- sf::st_transform(ashe, 6542)
> get_landsat_imagery(
+ aoi = projected_ashe,
+ start_date = "2021-06-01",
+ end_date = "2021-06-30",
+ output_filename = tempfile(fileext = ".tif")
+ ) |>
+ terra::rast() |>
+ terra::plot()I have some minor discomforts about when exactly warping or masking is done. I just want to talk about the details of this and might follow up, I'm a bit lost in the depths of the compositing function and use of sprc and mosaic, though. A standalone question I had: can we use the aoi to drive the STAC query, but then download the files as-is without cropping/warping or compositing? I guess that would mean providing a link to the temp-file space being used. Fin. Thanks so much for such a great package, and much apology for being so late here (especially appreciative of the patience of @mikemahoney218 and @jhollist). |
|
Thank you so much, @mdsumner ! I'm excited to dive into your review. As I said to Jeff over email, I also haven't been a paragon of timeliness here -- I got started replying to Felipe three weeks ago, and then immediately lost my focus. I'm hoping to carve out time to respond to both reviews starting on Wednesday of this week! |
Response to @OldLipeThank you so, so much for your review here @OldLipe ! I feel like your comments have really improved the package. I'll walk through specific changes below, but first I wanted to ask about one remaining comment:
The issue I have with generating non-random names is that I feel users are in a better position than I am to know how to organize their files within a project. Users downloading multiple files are likely iterating through either collections, time ranges, or spatial areas of interest, and probably have a pre-existing idea of what distinguishes each file (and therefore would make for a good name). This is why, for instance, the documentation shows examples of providing your own So the idea behind random filenames is that it's something that is good enough for fast proof-of-concept "does this function work" tests, but is clearly not good enough for "real" usage. I'm hoping to force users to come up with their own file name conventions, rather than accepting the default options. This is also why I do handle the auto-naming when I'm curious what you think about this reasoning. If anything, I'd lean towards removing the default filenames altogether, but I do think they're useful for quick evaluation of the package.
I moved these into functions: Permian-Global-Research/rsi@5adacb2
I couldn't reproduce the warning (though I've seen it before, I just don't understand what triggers it) but I updated the documentation with a gdalwarp option to silence the warning: Permian-Global-Research/rsi@b1a15ba I also saved to a temporary file -- thanks! Because these examples don't run on CRAN, that one is a really easy thing to accidentally miss 😅 There were a few more of these here: Permian-Global-Research/rsi@fa7dc91
I left the error in place, because I want to explain why this function intentionally doesn't let users use arbitrary functions. But I added more documentation around it: a comment right above the
I added additional examples to this document, using multiple data-retrieving functions and showing how to work with band mapping objects: Permian-Global-Research/rsi@7b82f3d
This was a straight-up mistake; thank you. Fixed here: Permian-Global-Research/rsi@f499c95
Thank you for the examples here; without them, I absolutely never would have figured out how bitmasks work. I added a
Discussed in #636 (comment) Thank you again -- this was a massively helpful review. Let me know if I missed anything (or made anything worse by mistake 😄) |
|
@mdsumner Thank you for your review and no worries on the timeline. We are very much appreciative (and understanding) of the time that all of our reviewers dedicate to rOpenSci! Keep an eye out for revisions and once those are in, use our review template (https://devguide.ropensci.org/approval2template.html) to indicate your approval of the revisions or if other changes are needed. @OldLipe Thank you for your review as well! As you can see @mikemahoney218 has addressed your review. When you have a chance could you take a look at that and let us know if his revisions address your concerns or if you would like to see some additional changes. As mentioned above, please use the review template (https://devguide.ropensci.org/approval2template.html) for this. For both of you, can you provide me with a rough estimate of hours spent on the review? This is something we keep track of. Thank you all! |
jhollist
added
4/review(s)-in-awaiting-changes
and removed
3/reviewer(s)-assigned
labels
Response to @mdsumnerThank you so much, Mike! This was an incredibly useful review process. I've responded to your specific comments below:
I added a small mention of this to the README and a larger mention to the Downloading vignette: Permian-Global-Research/rsi@55ad0d9
I'm curious if you have ideas on an efficient way to do this. I've tried to stub out tests to do this, but am running into issues that I wrote rsi to fix some of the rough edges I found when downloading data sets, which means I'm effectively re-implementing rsi's download functions in the test itself to try and square the circle. To give a more concrete example: I can download the assets of an item using
Added to the "How can I" vignette: Permian-Global-Research/rsi@528e31f And to the README and the
Fixed in Permian-Global-Research/rsi@d0acb46 . The documentation was also just wrong here; this function needed a bbox, not an sfc object. I changed things so either works. You can tell this function was pulled out from
I'm not sure I entirely follow you here! Would you be able to give an example?
Unfortunately I don't think gdalwarp can handle complicated compositing yet: OSGeo/gdal#5176 With regards to simple composites ("latest pixel wins" style), this is one of the messiest parts of The output of that gets passed as Then this is where things get really silly: if And then we wind up calling this warp with multiple This is actually a lot less complicated than it used to be -- there used to be an All that said, I documented this a bit more here: With regards to rescaling via the warper -- I'm definitely interested in this, but I've seen some rather complex rescaling formulas in the wild that aren't just a simple scale and offset, which has made me a bit spooked. I think there's still some dark magic you can do by writing a VRT with a complex transform equation, but I don't know that I understand VRTs well enough to maintain a package that did that, right now.
I added an example of using
Fixed: Permian-Global-Research/rsi@280d513
Yeah, it's an easy function to get lost in. You can see my own notes here from the last time I was refactoring: The steps are usually querying, filtering down the returned results, (warping and) downloading the relevant items, masking them, compositing the outputs, and then rescaling. The warping is controlled by the That processing function just sets the Which then eventually gets passed to the actual download call: So that's warping and downloading handled: each asset is (usually) warped and downloaded separately. Each asset then gets masked independently: These are masked independently mostly to make the implementation easier, because now each asset is composited into a single file per asset: I didn't want to try and track if all assets existed in all items, or so on, and so we don't aggregate assets into items until after rescaling. As for compositing: there's three pathways here. The first one I discussed above: if files didn't need to get masked or rescaled, we composited them during the download stage and they skip the composite process entirely. The second one is if users specified The third is for all the other functions, which are applied using As far as I'm concerned, Hope this all made sense!
I added a section to the "How Can I" vignette about one version of this -- downloading each item separately: (To skip masking, you'd also set If you want to get each asset separately, that's probably where |
|
Ok amazing, love these responses and the changes you've made. I think you'll need to link to this issue in the Details section, because it's a really great section on the concerns and the journey you've been on. (I'm getting more enmeshed in the python side via odc and so each time I come to rsi I have more perspective and learn a lot more). There's no showstoppers now from my perspective, I think you've responded to this review brilliantly and I'm stoked with all the updates you've made, and the explanations. Please consider my take as 'approved'. @jhollist
I actually didn't mean automated testing validation, just like a real world example where we can get confirmation of the values we see in a small context. I will follow up when I can but I don't consider this a blocker or anything. Also I need to follow up here (which I can't remember exactly now, I may have been thinking about a different part of the help content). When I explore again I will bring this up, outside this review as an issue/discussion piece.
Thanks!! |
|
Looks like we are really close on this one! @OldLipe do you feel like to @mikemahoney218 has addressed the issues raised in your review? @mdsumner Thank you for the follow up and the approval! How many hours do you think you spent on the review? You both can use this template for your response: https://devguide.ropensci.org/approval2template.html or you can just provide that directly as well! |
|
@mdsumner and @OldLipe Just trying to finalize this. See #636 (comment) |
jhollist
added
5/awaiting-reviewer(s)-response
and removed
4/review(s)-in-awaiting-changes
labels
|
7 hours was my tally 🙏 |
|
@ropensci-review-bot submit review #636 (comment) time 7 |
|
Logged review for mdsumner (hours: 7) |
|
@ropensci-review-bot submit review #636 (comment) time 5 |
|
Logged review for OldLipe (hours: 5) |
|
@OldLipe thanks for the email. I am recording your response and acceptance of @mikemahoney218 revisions here. No need for you to do it as well. And I think we are all set to go. Thank you all for your efforts on this! Will work on moving this along later this AM. |
|
@ropensci-review-bot approve rsi |
|
Approved! Thanks @mikemahoney218 for submitting and @mdsumner, @OldLipe for your reviews! 😁 To-dos:
Should you want to acknowledge your reviewers in your package DESCRIPTION, you can do so by making them Welcome aboard! We'd love to host a post about your package - either a short introduction to it with an example for a technical audience or a longer post with some narrative about its development or something you learned, and an example of its use for a broader readership. If you are interested, consult the blog guide, and tag @ropensci/blog-editors in your reply. They will get in touch about timing and can answer any questions. We maintain an online book with our best practice and tips, this chapter starts the 3d section that's about guidance for after onboarding (with advice on releases, package marketing, GitHub grooming); the guide also feature CRAN gotchas. Please tell us what could be improved. Last but not least, you can volunteer as a reviewer via filling a short form. |
|
Thank you so much @jhollist , @mdsumner , and @OldLipe ! This was a fantastic process (as usual with rOpenSci!). @jhollist -- I mentioned at the start of the review, but I'm not able to transfer this repo to the rOpenSci namespace. I'm not sure which boxes on the checklist still apply -- and I've got the faintest memory that the package needs to get added to a registry somewhere, but I forget where that is. Does this make sense? |
|
👋 here! ropensci/roregistry@e5d2c02 should be it but I'll be checking the package and registry building to be sure. 😸 |
|
Registry now updated as expected. All good! |
|
@mpadge and @maelle Is there anything special that @mikemahoney218 needs to do for this since he won't be transferring the package to the rOpenSci org? Looks like things have moved along fine without that. |
Date accepted: 2024-10-01
Submitting Author Name: Mike Mahoney
Submitting Author Github Handle: @mikemahoney218
Repository: https://github.com/Permian-Global-Research/rsi/
Version submitted:
Submission type: Standard
Editor: @jhollist
Reviewers: @mdsumner, @OldLipe
Archive: TBD
Version accepted: TBD
Language: en
Scope
Please indicate which category or categories from our package fit policies this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.):
Explain how and why the package falls under these categories (briefly, 1-2 sentences):
This package supports (spatial) data retrieval from APIs implementing the OGC STAC API standard, processing the downloaded data (including automated masking, compositing and rescaling) computing spectral indices from those data, and wrangling the outputs into formats useful for modeling and visualization.
Anyone with a need to download and process spatial data, particularly remote sensing data, particularly satellite-based earth observation rasters. We've used rsi to automate the entire data preparation process of forest carbon and structure models (not yet published), but the package is broadly useful to anyone working in Earth surface modeling.
Yes:
There are a few minor things that I think rsi does better than other approaches (we sign items right before each one is downloaded, for instance, whereas some other packages sign items before starting to download the entire set, meaning the signature can expire causing large downloads to fail), but this difference I think is mostly a matter of taste. I'm very familiar with local GDAL, terra, and sf, and so rsi tries to get users back to working with local GDAL, terra, and sf as fast as possible.
NA
NA
pkgcheckitems which your package is unable to pass.I have never successfully gotten the CI item to be a check, and I have no idea why. I'm using the standardApparently this is a local-only issue.usethisfunctions to structure my CI, for what it's worth!I addressed failing covr in #636 (comment)
Technical checks
Confirm each of the following by checking the box.
This package:
Publication options
Do you intend for this package to go on CRAN?
Do you intend for this package to go on Bioconductor?
Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:
MEE Options
Code of conduct
The text was updated successfully, but these errors were encountered: