ropensci/software-review

Submission: MODIStsp

lbusett opened this issue · 56 comments

Summary

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

Facilitates the creation of time series of raster images derived from MODIS Satellite Land Products by automating and standardizing several preprocessing steps (e.g., download, mosaicing, reprojection, resize, indexes computation, data extraction). Processing parameters are set using a powerful GUI. Non-interactive execution using saved Options Files allows updating time series when new images are available using cron jobs.

  • Paste the full DESCRIPTION file inside a code block below:
Package: MODIStsp
Title: A Tool for Automating Download and Preprocessing of MODIS Land Products
    Data
Type: Package
Version: 1.3.3.9000
Authors@R: c(person("Lorenzo", "Busetto", email = "lbusett@gmail.com", role = c("aut", "cre"),
    comment = c(ORCID = '0000-0001-9634-6038')), 
    person("Luigi", "Ranghetti", email = "ranghetti.l@irea.cnr.it", role = c("aut")))
Description: Allows automating the creation of time series of rasters derived
    from MODIS Satellite Land Products data. It performs several typical
    preprocessing steps such as download, mosaicking, reprojection and resize
    of data acquired on a specified time period. All processing parameters
    can be set using a user-friendly GUI. Users can select which layers of
    the original MODIS HDF files they want to process, which additional
    Quality Indicators should be extracted from aggregated MODIS Quality
    Assurance layers and, in the case of Surface Reflectance products
    , which Spectral Indexes should be computed from the original reflectance
    bands. For each output layer, outputs are saved as single-band raster
    files corresponding to each available acquisition date. Virtual files
    allowing access to the entire time series as a single file are also created.
    Command-line execution exploiting a previously saved processing options
    file is also possible, allowing to automatically update time series
    related to a MODIS product whenever a new image is available.
License: GPL-3
Depends:
    R (>= 3.1.3)
Imports:
    bitops (>= 1.0-6),
    data.table (>= 1.9.6),
    gdalUtils (>= 2.0.1.7),
    gWidgets (>= 0.0-54),
    gWidgetsRGtk2,
    methods,
    httr (>= 1.1.0),
    jsonlite,
    pacman,
    parallel,
    raster (>= 2.5-2),
    RCurl (>= 1.95-4.8),
    rgdal (>= 1.0-3),
    rgeos (>= 0.3-8),
    sp (>= 1.2-2),
    stringr (>= 1.0.0),
    xts (>= 0.9-7),
    xml2
Suggests:
    spelling,
    knitr,
    rmarkdown,
    png,
    grid,
    httptest,
    covr
SystemRequirements: Cairo >= 1.0.0, ATK (>= 1.10.0), Pango (>= 1.10.0), GTK+ (>=
    2.8.0), GLib (>= 2.8.0), Curl, GDAL (>= 1.6.3), PROJ.4 (>= 4.4.9)
URL: https://github.com/lbusett/MODIStsp
BugReports: https://github.com/lbusett/MODIStsp/issues
LazyData: true
VignetteBuilder: knitr
RoxygenNote: 6.0.1
Roxygen: list(markdown = TRUE)

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

    https://github.com/lbusett/MODIStsp

  • 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, because the package facilitates access to MODIS satellite data;
    • geospatial data, because MODIS time series are widely used by the scientific community for several geospatial applications
  •   Who is the target audience and what are scientific applications of this package?  

    The target audience includes anyone involved/interested in monitoring the properties of the Earth surface through the analysis of time series of coarse resolution satellite imagery.
    MODIS satellite data are in fact used in many different scientific applications, since they allow global daily monitoring of the earth surface. Their applications are therefore too many to analyze in this context. Among the main ones, I can cite (borrowing a bit from wikipedia...) the monitoring of vegetation health by means of time-series analyses with vegetation indices, the analysis of long term land cover changes (e.g. to monitor deforestation rates), real time monitoring of snow/ice cover, analysis of water quality and of impacts of floodings, change of water levels of major lakes and the detection and mapping of wildland fires. MODIS data are also extensively used as ancillary input in modelling applications (e.g., biogeochemical models for Primary Production modelling; crop yield models). Additionally, MODIS data are often used by biologists/zoologists as ancillary data to monitor/model migratory patterns.

    (Introductory material concerning MODIS and its main applications can be found for example here and here)

  • Are there other R packages that accomplish the same thing? If so, how does
    yours differ or meet our criteria for best-in-category?

    The main “R” packages overlapping MODIStsp functionality are MODIS and MODISTools
    MODIStsp and MODIS have substantial overlaps, since they are both meant to provide access and preprocessing to MODIS products. Two aspects that in my opinion differentiate MODIStsp are its user-friendliness (I think that the MODIStsp Graphical User Interface is very helpful, in particular for less skilled "R" users, though I am clearly a bit biased on the topic), and its ability to allow on-the-fly creation of time series of standard/user provided Spectral Indexes (often used for time series analysis) and of MODIS quality indicators. On the other hand, MODIS provides some additional post-processing functionality (e.g., filtering/smoothing), which is something I'm planning to add in the near future.

    MODIStools is a very nice package, but it provides somewhat more limited functionality. It is based on the ORNL DAAC webservice (https://modis.ornl.gov/data/modis_webservice.html), and therefore provides access only to some MODIS products. Additionally, it is meant to allow access to data concerning a given set of points and their surroundings, while both MODIStsp and MODIS are meant to provide downloading capabilitites to the "full" MODIS raster data (or subsets of those, although the full HDFs need always to be downloaded).

    The ModisDownload function of package rts is also overlapping, but its functionalities are more limited than either MODIStsp and MODIS. (Actually, an early version of ModisDownload is what got us started in developing the package - proper acknwoledgement is given in MODIStsp documentation).

    Other packages providing access to MODIS data include MODISSnow (limited to snow cover products distributed by NSIDC) and modiscloud (which provides tools for processing Level 2 Cloud Mask SWATH products – not supported by MODIStsp)
    For further reference, our paper concerning MODIStsp published on CAGEO provides in Section 1 a short comparison of the different tools currently available for accessing MODIS data (both "R" based or not).

  •   If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted.

    #143

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.

    NOTE ON INSTALLATION Installation (both from github or from CRAN) may be a bit problematic due the fact that MODIStsp requires the gWidgetsRGtk2 library to build its GUI, which often have installation problems on Windows and Mac due to requiring the GTK+ library, and needing several packages to be installed beforehand on Linux. In case of problems, it is better to install gWidgetsRGtk2 beforehand. Specific instructions are provided at (http://lbusett.github.io/MODIStsp/articles/installation.html)

  • 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, Coveralls 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?
    The package is already on CRAN (https://cran.r-project.org/package=MODIStsp)

  • 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)
  • Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:

    • The package is novel and will be of interest to the broad readership of the journal.
    • The manuscript describing the package is no longer than 3000 words.
    • You intend to archive the code for the package in a long-term repository which meets the requirements of the journal.
    • (Please do not submit your package separately to Methods in Ecology and Evolution)

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:

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

Michael Sumner - @mdsumner
Jeff Hanson - @jeffreyhanson
Leah Wasser - @lwasser
Robin Lovelace - @robin

hey @lbusett one of my students used this package in his final project in the fall! he showed it to me and said it was really nice.

👋 @lbusett I will be handling your submission. Thanks also for the reviewer suggestions.
@lwasser Would you be interested in being one of the reviewers?

I have also reached out to @jeffreyhanson by email.

@karthik Great, thanks!
@lwasser Glad to hear about that!

sure @karthik i'll review this. i wanted to spend some time playing with it after my student showed it to me. What is the deadline? i'll need a few weeks given i just got back from travel today.

Sure, I'm happy to review this. I'm pretty busy at the moment though, so it might be a few weeks before I can get around to it if that's ok?

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

Work in progress (dealing with installation issues)


Reviewers: @lwasser and @jeffreyhanson
Due date: Feb 10, 2018

@lwasser @jeffreyhanson Thanks very much! I've tentatively assigned Feb 10th. Please let me know if anything changes. 🙏

@karthik Sorry to hear that you having problems in installation.... 😟 Unfortunately, as I reported in the submission, that is a common problem related to the requirements of the gWidgetsRGtk2 package. Instructions reported here: http://lbusett.github.io/MODIStsp/articles/installation.html are usually sufficient to solve the issue (unless you are working on a mac with os sierra, in which case it can be more dificult...).

Let me know if I can help in any way!

Hi @karthik . Just wanted to ask if in the end you managed to solve the installation issues or you are still having problems. In case you are working on a mac and you are still struggling, these are the "latest" instructions I found regarding how to install packages "RGtk2" (and consequently gWidgetsRGtk2) on mac os Sierra:

https://stackoverflow.com/a/46748516/6871135

Awesome work @lbusett! This package looks absolutely amazing. I've just started working on my review, but I thought I would let you know that some of the unit tests fail on my system in case you wanted to address this before I submit my review. I've included the output below and a copy of my session info.

> test()
Loading MODIStsp
Testing MODIStsp
✔ | OK F W S | Context
✔ |  4       | Check proper functioning of bbox_from_file [1.6 s]
✔ |  4       | get_yeardates works as expected when download_range == "Full"
✔ |  6       | get_yeardates works as expected when download_range == "Seasonal"
✔ |  7       | Check proper functioning of MODIStsp_addindex [0.2 s]
✔ | 21       | MODIStsp_extract works as expected [58.7 s]
⠏ |  0       | Check proper functioning of the main MODIStsp GUI
(R:19949): Gtk-WARNING **: Failed to set text from markup due to error parsing markup: Error on line 1 char 83: Element 'markup' was closed, but the currently open element is 'span'

(R:19949): Gtk-WARNING **: Failed to set text from markup due to error parsing markup: Error on line 1 char 83: Element 'markup' was closed, but the currently open element is 'span'

(R:19949): Gtk-WARNING **: Failed to set text from markup due to error parsing markup: Error on line 1 char 87: Element 'markup' was closed, but the currently open element is 'span'

(R:19949): Gtk-WARNING **: Failed to set text from markup due to error parsing markup: Error on line 1 char 87: Element 'markup' was closed, but the currently open element is 'span'

(R:19949): Gtk-WARNING **: Failed to set text from markup due to error parsing markup: Error on line 1 char 83: Element 'markup' was closed, but the currently open element is 'span'

(R:19949): Gtk-WARNING **: Failed to set text from markup due to error parsing markup: Error on line 1 char 83: Element 'markup' was closed, but the currently open element is 'span'

(R:19949): Gtk-WARNING **: Failed to set text from markup due to error parsing markup: Error on line 1 char 83: Element 'markup' was closed, but the currently open element is 'span'

(R:19949): Gtk-WARNING **: Failed to set text from markup due to error parsing markup: Error on line 1 char 83: Element 'markup' was closed, but the currently open element is 'span'
✔ |  0       | Check proper functioning of the main MODIStsp GUI [4.1 s]
✔ |  6   2   | MODIStsp Test 0: Gracefully fail on input problems [14.9 s]
────────────────────────────────────────────────────────────────────────────────
test_MODIStsp.R:13: warning: Tests on MODIStsp
input string 1 is invalid in this locale

test_MODIStsp.R:18: warning: Tests on MODIStsp
input string 1 is invalid in this locale
────────────────────────────────────────────────────────────────────────────────
⠏ |  0       | MODIStsp Test 1: Basic processing on bands and quality
⠋ |  1       | MODIStsp Test 1: Basic processing on bands and quality
⠙ |  2       | MODIStsp Test 1: Basic processing on bands and quality
✔ |  2       | MODIStsp Test 1: Basic processing on bands and quality
        indicators [5.3 s]
✔ |  4       | MODIStsp Test 2: Geometric operations [3.2 s]
⠏ |  0       | MODIStsp Test 3: Computation of spectral indices and
            creation of time seriesWarning 1: Metadata exceeding 32000 bytes cannot be written into GeoTIFF. Transferred to PAM instead.
⠋ |  1       | MODIStsp Test 3: Computation of spectral indices and
⠙ |  2       | MODIStsp Test 3: Computation of spectral indices and
⠹ |  3       | MODIStsp Test 3: Computation of spectral indices and
⠸ |  4       | MODIStsp Test 3: Computation of spectral indices and
⠼ |  5       | MODIStsp Test 3: Computation of spectral indices and
            creation of time seriesWarning 1: Metadata exceeding 32000 bytes cannot be written into GeoTIFF. Transferred to PAM instead.
⠴ |  6       | MODIStsp Test 3: Computation of spectral indices and
⠦ |  7       | MODIStsp Test 3: Computation of spectral indices and
✔ |  7       | MODIStsp Test 3: Computation of spectral indices and
            creation of time series [22.0 s]
✖ |  0 1     | MODIStsp Test 4: HTTP download from NSIDC (seasonal) [5.8 s]
────────────────────────────────────────────────────────────────────────────────
test_MODIStsp.R:193: error: Tests on MODIStsp
argument is of length zero
1: MODIStsp(test = 4) at /home/jeff/GitHub/MODIStsp/tests/testthat/test_MODIStsp.R:193
2: MODIStsp_process(sel_prod = general_opts$sel_prod, start_date = general_opts$start_date, 
       end_date = general_opts$end_date, out_folder = general_opts$out_folder, out_folder_mod = general_opts$out_folder_mod, 
       reprocess = general_opts$reprocess, delete_hdf = general_opts$delete_hdf, sensor = general_opts$sensor, 
       download_server = general_opts$download_server, user = general_opts$user, password = general_opts$password, 
       https = prod_opts$http, ftps = prod_opts$ftp, start_x = general_opts$start_x, 
       start_y = general_opts$start_y, end_x = general_opts$end_x, end_y = general_opts$end_y, 
       full_ext = general_opts$full_ext, bbox = general_opts$bbox, out_format = general_opts$out_format, 
       out_res_sel = general_opts$out_res_sel, out_res = as.numeric(general_opts$out_res), 
       native_res = prod_opts$native_res, tiled = prod_opts$tiled, resampling = general_opts$resampling, 
       ts_format = general_opts$ts_format, compress = general_opts$compress, mod_proj_str = mod_proj_str, 
       outproj_str = general_opts$user_proj4, nodata_in = prod_opts$nodata_in, nodata_out = prod_opts$nodata_out, 
       rts = general_opts$rts, nodata_change = general_opts$nodata_change, scale_val = general_opts$scale_val, 
       scale_factor = prod_opts$scale_factor, offset = prod_opts$offset, datatype = prod_opts$datatype, 
       bandsel = general_opts$bandsel, bandnames = prod_opts$bandnames, indexes_bandsel = c(general_opts$indexes_bandsel), 
       indexes_bandnames = c(prod_opts$indexes_bandnames, custom_idx$indexes_bandnames), 
       indexes_formula = c(prod_opts$indexes_formula, custom_idx$indexes_formulas), 
       indexes_nodata_out = c(prod_opts$indexes_nodata_out, custom_idx$indexes_nodata_out), 
       quality_bandnames = prod_opts$quality_bandnames, quality_bandsel = general_opts$quality_bandsel, 
       quality_bitN = prod_opts$quality_bitN, quality_source = prod_opts$quality_source, 
       quality_nodata_in = prod_opts$quality_nodata_in, quality_nodata_out = prod_opts$quality_nodata_out, 
       file_prefixes = prod_opts$file_prefix, main_out_folder = prod_opts$main_out_folder, 
       gui = gui, use_aria = general_opts$use_aria, download_range = general_opts$download_range, 
       n_retries = n_retries, verbose = verbose) at /home/jeff/GitHub/MODIStsp/R/MODIStsp.R:464
3: MODIStsp_download(modislist, out_folder_mod, download_server, http, ftp, n_retries, 
       use_aria, date_dirs[date], year, DOY, user, password, sens_sel, date_name, gui, 
       mess_lab, verbose) at /home/jeff/GitHub/MODIStsp/R/MODIStsp_process.R:388
────────────────────────────────────────────────────────────────────────────────
✖ |  0 1     | MODIStsp Test 5: HTTP download from USGS, resize and reproject [14.5 s]
────────────────────────────────────────────────────────────────────────────────
test_MODIStsp.R:249: error: Tests on MODIStsp
argument is of length zero
1: MODIStsp(test = 5) at /home/jeff/GitHub/MODIStsp/tests/testthat/test_MODIStsp.R:249
2: MODIStsp_process(sel_prod = general_opts$sel_prod, start_date = general_opts$start_date, 
       end_date = general_opts$end_date, out_folder = general_opts$out_folder, out_folder_mod = general_opts$out_folder_mod, 
       reprocess = general_opts$reprocess, delete_hdf = general_opts$delete_hdf, sensor = general_opts$sensor, 
       download_server = general_opts$download_server, user = general_opts$user, password = general_opts$password, 
       https = prod_opts$http, ftps = prod_opts$ftp, start_x = general_opts$start_x, 
       start_y = general_opts$start_y, end_x = general_opts$end_x, end_y = general_opts$end_y, 
       full_ext = general_opts$full_ext, bbox = general_opts$bbox, out_format = general_opts$out_format, 
       out_res_sel = general_opts$out_res_sel, out_res = as.numeric(general_opts$out_res), 
       native_res = prod_opts$native_res, tiled = prod_opts$tiled, resampling = general_opts$resampling, 
       ts_format = general_opts$ts_format, compress = general_opts$compress, mod_proj_str = mod_proj_str, 
       outproj_str = general_opts$user_proj4, nodata_in = prod_opts$nodata_in, nodata_out = prod_opts$nodata_out, 
       rts = general_opts$rts, nodata_change = general_opts$nodata_change, scale_val = general_opts$scale_val, 
       scale_factor = prod_opts$scale_factor, offset = prod_opts$offset, datatype = prod_opts$datatype, 
       bandsel = general_opts$bandsel, bandnames = prod_opts$bandnames, indexes_bandsel = c(general_opts$indexes_bandsel), 
       indexes_bandnames = c(prod_opts$indexes_bandnames, custom_idx$indexes_bandnames), 
       indexes_formula = c(prod_opts$indexes_formula, custom_idx$indexes_formulas), 
       indexes_nodata_out = c(prod_opts$indexes_nodata_out, custom_idx$indexes_nodata_out), 
       quality_bandnames = prod_opts$quality_bandnames, quality_bandsel = general_opts$quality_bandsel, 
       quality_bitN = prod_opts$quality_bitN, quality_source = prod_opts$quality_source, 
       quality_nodata_in = prod_opts$quality_nodata_in, quality_nodata_out = prod_opts$quality_nodata_out, 
       file_prefixes = prod_opts$file_prefix, main_out_folder = prod_opts$main_out_folder, 
       gui = gui, use_aria = general_opts$use_aria, download_range = general_opts$download_range, 
       n_retries = n_retries, verbose = verbose) at /home/jeff/GitHub/MODIStsp/R/MODIStsp.R:464
3: MODIStsp_download(modislist, out_folder_mod, download_server, http, ftp, n_retries, 
       use_aria, date_dirs[date], year, DOY, user, password, sens_sel, date_name, gui, 
       mess_lab, verbose) at /home/jeff/GitHub/MODIStsp/R/MODIStsp_process.R:388
────────────────────────────────────────────────────────────────────────────────
  |======================================================================| 100%
  |======================================================================| 100%
  |======================================================================| 100%
  |======================================================================| 100%
✔ |  6       | MODIStsp Test 6: FTP download and mosaicing of MODIS tiles [77.0 s]
✔ |  1       | MODIStsp Test 7: Passing the extent with a spatial file [3.0 s]
⠏ |  0       | MODIStsp Test 8: Fail gracefully on missing connectionError : GET http://e4ftl01.cr.usgs.gov/MOTA/MCD43A3.006/
In addition: Warning message:
`encoding` is deprecated; all files now assumed to be UTF-8 
Error : GET ftp://ladsweb.nascom.nasa.gov/allData/6/MCD43A3/2017/
⠋ |  1       | MODIStsp Test 8: Fail gracefully on missing connectionError : GET ftp://ladsweb.nascom.nasa.gov/allData/6/MCD15A2H/2016/
✔ |  2       | MODIStsp Test 8: Fail gracefully on missing connection [0.5 s]
✖ |  0 1     | Check proper functioning of MODIStsp_readxml [1.9 s]
────────────────────────────────────────────────────────────────────────────────
test_MODIStsp_readxml.R:8: error: MODIStsp_readxml works as expected
attempt to select less than one element in OneIndex
1: MODIStsp_read_xml(tmpfile, system.file("ExtData", "MODIStsp_ProdOpts.xml", package = "MODIStsp")) at /home/jeff/GitHub/MODIStsp/tests/testthat/test_MODIStsp_readxml.R:8
────────────────────────────────────────────────────────────────────────────────
✔ |  2       | MODIStsp_resetindexes

══ Results ═════════════════════════════════════════════════════════════════════
Duration: 213.2 s

OK:       72
Failed:   3
Warnings: 2
Skipped:  0

Session information

R version 3.4.3 (2017-11-30)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 16.04.3 LTS

Matrix products: default
BLAS: /usr/lib/libblas/libblas.so.3.6.0
LAPACK: /usr/lib/lapack/liblapack.so.3.6.0

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
 [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] httptest_3.0.0       gWidgetsRGtk2_0.0-85 cairoDevice_2.24    
[4] gWidgets_0.0-54      RGtk2_2.20.34        MODIStsp_1.3.3.9000 
[7] testthat_2.0.0       devtools_1.13.4     

loaded via a namespace (and not attached):
 [1] Rcpp_0.12.15        compiler_3.4.3      R.methodsS3_1.7.1  
 [4] bitops_1.0-6        R.utils_2.6.0       iterators_1.0.9    
 [7] tools_3.4.3         xts_0.10-1          gdalUtils_2.0.1.7  
[10] digest_0.6.15       jsonlite_1.5        memoise_1.1.0      
[13] lattice_0.20-35     rlang_0.1.6         foreach_1.4.4      
[16] cli_1.0.0           commonmark_1.4      curl_3.1           
[19] rgdal_1.2-16        parallel_3.4.3      withr_2.1.1        
[22] stringr_1.2.0       httr_1.3.1          roxygen2_6.0.1     
[25] raster_2.6-7        xml2_1.2.0          rgeos_0.3-26       
[28] grid_3.4.3          data.table_1.10.4-3 R6_2.2.2           
[31] pacman_0.4.6        sp_1.2-7            magrittr_1.5       
[34] codetools_0.2-15    assertthat_0.2.0    mime_0.5           
[37] stringi_1.1.6       RCurl_1.95-4.10     crayon_1.3.4       
[40] zoo_1.8-1           R.oo_1.21.0  

Let me know if there's any other info I can provide.

@jeffreyhanson

Thanks! Glad you like our package!

Concerning the tests, thanks for reporting the issues. They should now be fixed. One was due to a recent change in xml2 functionality, the other (apparently) to a change in the "standard" http response of NASA servers. If you pull the new github version, the tests should pass (and you should also be able to download data from http also outside of the tests, which I think ws not possible due to the NASA response change. I'll have to also test the CRAN version to see if it still works over http.

hey @lbusett or @jeffreyhanson i'm a bit confused on the quartz install
screen shot 2018-02-05 at 11 33 57 am

i'm here:

next, edit the configure options for GTK to require x11 rather than Quartz:

brew edit gtk+

in the def install section, remove the reference to quartz and switch to:

--with-gdktarget=x11,
--enable-x11-backend

And would like to understand what part of the gtk+ file i need to edit. do i just replace "with-gdktarget=quartz", with two lines as follows:

"--with-gdktarget=x11",
"--enable-x11-backend",

it's taking me some time to get everything installed and this step is a bit confusing. thank you for any advice!

So i'm not sure if i did this right but another question. Does this message make sense:

library(RGtk2)
R session is headless; GTK+ not initialized.

hi @lwasser ,

sorry you are having trouble... unfortunately, at the moent RGtk2 installation on mac is troublesome (and the solution to the trouble seems to change with time.. ;-(.
Moreover, I know almost nothing about mac, so I admit that I never really tested the instructions provided in the help.... However, those instructions mean that indeed you need to modify the gtk file so that it reads:

def install
args = ["--disable-dependency-tracking",
"--disable-silent-rules",
"--prefix=#{prefix}",
"--disable-glibtest",
"--enable-introspection=yes",
"--with-gdktarget=x11",
"--enable-x11-backend"
]

and then proceed with brew install.

Concerning the "R session is headless; GTK+ not initialized." : this does not bode well. It (should) mean that RGtk2 does not find a "graphical environment" to use. I fear that if you try to load gWidgetsRGtk2 you will have problems.

Again, sorry for the trouble. I hope you'll be able to solve it easily (this link may also help : https://gist.github.com/sebkopf/9405675). Otherwise, I'll try to get my hands on a (modern) mac pc and see if I manage to provide better instructions.

ok so remove "--disable-visibility" as well?
Yes i'm having many issues now. i can't seem to get cairoDevice or gWidgetsRGtk2 to install correctly. i'm not sure what to try next to get them to install.

> install.packages("cairoDevice", lib="~/Library/Frameworks/R.framework/Versions/3.4/Resources/library/cairoDevice")
Warning in install.packages :
  'lib = "~/Library/Frameworks/R.framework/Versions/3.4/Resources/library/cairoDevice"' is not writable
Would you like to use a personal library instead?  (y/n) y
Package which is only available in source form, and may need compilation of
  C/C++/Fortran: ‘cairoDevice’
Do you want to attempt to install these from sources?
y/n: y
installing the source package ‘cairoDevice’

trying URL 'https://cran.rstudio.com/src/contrib/cairoDevice_2.24.tar.gz'
Content type 'application/x-gzip' length 39639 bytes (38 KB)
==================================================
downloaded 38 KB

* installing *source* package ‘cairoDevice’ ...
** package ‘cairoDevice’ successfully unpacked and MD5 sums checked
checking for pkg-config... pkg-config
ERROR: gtk+2.0 not found by pkg-config.
ERROR: configuration failed for package ‘cairoDevice’
* removing ‘/Users/lewa8222/Library/R/3.4/library/cairoDevice’
Warning in install.packages :
  installation of package ‘cairoDevice’ had non-zero exit status

The downloaded source packages are in
	‘/private/var/folders/43/4q82487d5xsfpxdx6nl_c1wmhckx08/T/RtmpJoK8cV/downloaded_packages’

does this help at all with troubleshooting? if i say "n" to install from source it just quits. i'm wondering if i get this error because i edited the install code above incorrectly (i left disable-visibility in the code).

Just a check:

did you already do

export PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/usr/local/lib/pkgconfig/gtk+-2.0.pc:/opt/X11/lib/pkgconfig

which I think is setting the path to gtk+ and should solve to cairo installation issue (ERROR: gtk+2.0 not found by pkg-config.)?

ok!! i think i just got it. THANK YOU @lbusett i just installed those packages manually but also i hadn't run that line above that you suggested so i did that too. phew! i'll play with the package more tomorrow. thank you for the help.

Sorry for the late reply @lbusett, I just installed the latest GitHub version and all the tests pass now. Thank for you fixing this so quickly.

Good. In the end, both problems were due to a breaking change in xml2. Since the cran version uses xml, I was not notified about it.

By the way: a user signalled us a bug in the gui that I probably introduced while refactoring the code for ropensci submission. I am going to commit a fix for it ASAP.

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

  • [:white_check_mark:] 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:

  • [:white_check_mark:] A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • [:white_check_mark:] Installation instructions: for the development version of package and any non-standard dependencies in README
  • [:white_check_mark:] Vignette(s) demonstrating major functionality that runs successfully locally
  • [:white_check_mark:] Function Documentation: for all exported functions in R help
  • [:white_check_mark:] Examples for all exported functions in R Help that run successfully locally
  • [:white_check_mark:] 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

  • [:white_check_mark:] Installation: Installation succeeds as documented.
  • [:white_check_mark:] Functionality: Any functional claims of the software been confirmed.
  • [:white_check_mark:] Performance: Any performance claims of the software been confirmed.
  • [:white_check_mark:] 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.
  • [:x:] 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:


Review Comments

The MODIStsp package is absolutely brilliant. In my experience, downloading and pre-processing raw MODIS data can be extremely painful. The MODIStsp package automates this; meaning that users can spend less time wrangling data and more time analyzing data! This package also provides a graphical user interface too! The package documentation is comprehensive and well written. It contains several vignettes that cover installation, interactive and non-interactive usage, and frequently asked questions. The package website is stunning. I think this R package will excite both new and advanced R users. All in all, MODIStsp is a wonderful piece of open source software.

Major issues

  1. A lot of the examples and vignettes require the user to input their own path to store the data. I would find it really useful if these examples just defaulted to using the temporary directory (i.e. base::tempdir()) so I could run them by copying and pasting code.

Minor issues

  1. Running devtools::document() yields the following warnings:
1: In tools::parse_Rd(path) :
  /home/jeff/GitHub/MODIStsp/man/install_MODIStsp_launcher.Rd:20: unknown macro '\Progra'
2: In tools::parse_Rd(path) :
  /home/jeff/GitHub/MODIStsp/man/install_MODIStsp_launcher.Rd:20: unknown macro '\bin'
  1. It is my understanding that recent changes to the CRAN policies indicate that example code should write to the default temporary directory. If my understanding is correct, then the MODIStsp_extract example should use base::tempdir() instead of writing to base::system.file("testdata/", package = "MODIStsp"). I have copied the relevant text from the CRAN policies below:
Packages should not write in the user's home filespace (including clipboards), nor anywhere else on the 
file system apart from the R session’s temporary directory (or during installation in the location pointed to 
by TMPDIR: and such usage should be cleaned up). Installing into the system’s R installation (e.g., 
scripts to its bin directory) is not allowed.
  1. The references in vignettes/MODIStsp.pdf are not shown correctly. They contain text like "[-@Hengl2010]" instead of the in-text reference.

ROpensci packaging guidelines

  1. This package uses RCurl. From what I can tell, the only usage of RCurl is `RCurl::getURL (line 132 in R/MODIStsp_download.R) so you might be able to drop this dependency without too much hassle.
  2. This package uses base::print instead of base::message in a couple of places. This only happens in two places (lines 236 & 334 in R/MODIStsp_extract.R) so you might be able replace this with base::message fairly easily.

@jeffreyhanson Thank for your very positive review! All your comments make sense and can be implemented fairly easily. I'll wait however also for the review from @lwasser and then address all comments in one go.

@lwasser if you are still working on this, please note that I just committed a new version to github, solving a recently discovered bug which broke the GUI when a user defined projection was selected (ropensci/MODIStsp#109). Apparently, I introduced that bug while refactoiring the code for ropensci. Unfortunately, it is almost impossible to develop unit tests for the GUI functions, making it difficult to debug them...

Thank you @lbusett i'm still working on the review. the install set me back this week - so sorry!
I hope to finish it today but i have to try to get it working on my laptop (i was on a desktop tuesday) for that to happen so I may have to submit this monday - worse case scenario. It's on my radar tho and the package itself is VERY VERY VERY COOL!

@lwasser Thanks for the update. No worries, really: we're not in a hurry! I just wanted to avoid for you to waste time on a buggy version!

All - so sorry this is a bit late. I hope it's helpful.

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

  • [x ] 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:

  • [x ] A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • [x ] Installation instructions: for the development version of package and any non-standard dependencies in README
  • [x ] Vignette(s) demonstrating major functionality that runs successfully locally
  • [ x] Function Documentation: for all exported functions in R help
  • [x ] Examples for all exported functions in R Help that run successfully locally
  • [ x] 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.
  • [ x] Functionality: Any functional claims of the software been confirmed.
  • [x ] 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. I DIDN"T RUN UNIT TESTS OR LOOK AT THIS ELEMENT CLOSELY
  • Packaging guidelines: The package conforms to the rOpenSci packaging guidelines SEE OTHER REVIEW!

Final approval (post-review)

  • [x ] The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 5


Review Comments

Overall I think this is a fantastic package. Anyone who has downloaded MODIS data knows that it can be a challenge. This allows for a fully reproducible workflow to grab the data. It’s also surprisingly fast! And I didn’t need my login. Hat tip to Lorenzo for creating such a useful package!

My notes below could be in some cases specific to my computer. I began my review my installing, and playing with the interface. I didn’t read the interface overview because I wanted to think about the intuitiveness of the interface itself FIRST. Then read documentation second!

Installation

I ran into significant issues installing this on my mac. I still don’t have it working on my laptop but did get it working on my desktop. It would be very nice to have one fully functional set of instructions for mac that is well tested! Here are a few notes:

A few notes:
Quartz needs to be installed first before you install cairo
Initially I ran into an issue with GTK+ not being installed.

This GIST was helpful
https://gist.github.com/sebkopf/9405675
Once I had GTK+ installed RGtk2 installed nicely

THIS Part was confusing to me.

Next, edit the configure options for GTK to require x11 rather than Quartz:
brew edit gtk+
in the def install section, remove the reference to quartz and switch to:
--with-gdktarget=x11,
--enable-x11-backend

Finally once I was able to get RGtk2 installed I got this message:

library(RGtk2)
R session is headless; GTK+ not initialized.

Once I installed these two (in installed from source just to be sure) that message seemed to go away

GDAL -- I didn’t have gdal setup on my desktop (only my laptop) so I had to install that too. Just a note that the instructions specify H4 support in GDAL and have command line instructions to check for this. However just note that sometimes gdal2 doesn’t play nicely on a mac!

See: GDAL2 link on a macif gdal isnt linked properly the command line functions don’t work. Just a note! This is tricky open source GIS mac space :)

Interface

Here I will be a bit nitpicky fully recognizing that this package is AWESOME. As a MAC user, I really don’t like quartz driven apps. It feels like the MAC interface is an afterthought and is clunky and the graphics are not crisp and are hard to read. If you’ve ever used ENVI on a mac you understand. It would be AWESOME to have a BETTER mac specific interface. Probably a ton of work but just better. My two cents.

With that said, the interface itself was overall very intuitive for me to work with. I was able to quickly get to some data without reading any documentation. However - if you want this package to be even more broadly used, a few more help buttons would be nice to include. More specifics are below:

It’s very nice to have all of this information in one spot vs the NASA website. However I wonder if we could make the interface more intuitive with a few other features.

Processing layers: I think what this does is allow me to select whatever products went into the derived product that I select. However, could you add a little help button to clarify that? Also when I select a few layers, there is no indication that I have selected anything in the gui. I can tell I selected a few things by hitting the “Click to Select” button again but could that maybe change in some way to suggest i’ve made a selection?

The product information page link is super awesome. I think that is too buried! I’m wondering if it could could go below the product box OR if the product box could have a link NEXT to it or something? It’s too nice of a feature to be so buried!! Also i’d make it a different color or button type since it’s a link rather than a selection. Maybe just text that is underlined in blue or something?
I’m confused by the add custom indexes button here as well. I will read more in the documentation but I wonder if this belongs here or if it could be added somewhere with some help as well to guide what you can add here. The hint is nice but I don’t think it’s enough for a user to really know what they can add / request here.

Now because i’m using quartz for this there is a weird quirk. When I have several boxes open (the select processing layers box and the insert new spectral index box) PLUS the original tsp interface... I have to figure out what order they are in so I can close them. Some users may think the entire thing is locked when really it’s just too many popups. This is a bit clunky for me to have 3 boxes and have to figure out the proper order! And it locked my computer up a bunch. I’m sure this is quartz specific but any way to control it?

Download Method
Next I come to the download method part of the interface. The help icons are very nice!
Spatial Extent
This part of the gui could confuse a user who doesn’t understand what a spatial extent is. In fact this is an area I helped my student with.

Show Tiles Map: This isn’t working properly for me. It spun and spun and did finally load but then I couldn’t click on it. It’s thus hard for me to understand how I select the extent UNLESS I have a numeric bounding box. Is there a way to streamline this aspect of the interface so you can create a box like you can on the website? And perhaps add more help buttons

Reprojection: A little help popup would be nice here that stats the default proj of MODIS and maybe a big FLAG associated with potential reprojection issues (my personal bias). Reprojection is something that overall so many people don’t understand. And so warning them it changes the data and introduces some error could be nice or just a link to a discussion about this.

Processing Options
I wonder if this should be called output file format or something like that. It may just be clearer to the user if you have output format options.

The Create raster stacks option is confusing to me. Does this create an R rasterstack object?
Finally i’m also a bit confused by the HDF file download section. I have 2 format options: ENVI and GTIFF. Yet below I have a save HDF file option. Am I able to select h5 somewhere? Or if this a general folder where the data will be saved? A few help buttons would be very helpful here. If this provides both Hdf and gtiff outputs that would be good to know or turn off if the user only wants gtiff.

Output
The output -- boy it was very fast grabbing the data !! I LOVE that I didn’t have to log in :)
What was returned however surprised me. I got what I think are asc files (.dat) with header files and then some hdf files. However I requested geotiff. Then there are R data files. I presume these contain the raster stacks.

I know this is a lot of feedback but overall i really think this is an excellent package! I’d just love for it a bit a bit more user friendly to us mac users :)

Non GUI Interface!

Boy is this nice. Fully automated and reproducible downloads. Here is a bit more nitpicking that you can take or leave! The documentation website is AWESOME! Thank you.

http://lbusett.github.io/MODIStsp/articles/noninteractive_execution.html

Vignettes
https://cran.r-project.org/web/packages/MODIStsp/vignettes/MODIStsp.pdf
Just a note that H4 and H5 can store raster data but are not inherently raster formats

WELL DONE! I hope there is slightly better mac support in a future iteration but overall i was impressed by how well the package worked!

@lwasser Thanks for your review! I think that your idea of going through the interface before reading the documentation was great: now I have a really useful analysis of a first-time user concerning the GUI implementation!.

I find most of your comments regarding the GUI very interesting and useful. I believe I can implement most of them reasonably easily (though I do not know if I can manage to do something regarding the xquartz interface on mac... sorry... ;-), while some others could be tricky and I have to think about them a bit more.

I will be out of office for a couple of days: when I am back, I'll dig into this a bit more, and probably reply with a list of what of your suggestions I can / plan to implement and which would be a bit of a mess...

Thanks again for a very useful review!

Lorenzo

PS: If you requested GEOTiff and got back .dat (which are ENVI files) there is definitely something strange happening. I'll check to see if I have introduced (another) bug while refactoring!

Quick note: I’ll be away from the internet for about a week and will be back online around 02/21. So I will be back to editorial activities shortly.

Hi all,

sorry for the late reply, but it was a very busy week.

Thanks again for your favourable and useful reviews. I just wanted to share some quick comments on the reviews before I start implementing changes:

Review 1 - @jeffreyhanson

Major issues
A lot of the examples and vignettes require the user to input their own path to store the data. I would find it really useful if these examples just defaulted to using the temporary directory (i.e. base::tempdir()) so I could run them by copying and pasting code.

Agreed. My "reason" for asking users to explicitly set an output folder was that "novice" users do not know about the existence of the R tempdir, so I feared that having to look for files in tempdir would confuse them. I can however explain how to do that in the examples / vignettes.

It is my understanding that recent changes to the CRAN policies indicate that example code should write to the default temporary directory. If my understanding is correct, then the MODIStsp_extract example should use base::tempdir() instead of writing to base::system.file("testdata/", package = "MODIStsp"). I have copied the relevant text from the CRAN policies below:

The MODIStsp_extract example (and test) use the base::system.file("testdata/", package = "MODIStsp" folder only to read some preprocessed data from that folder so that it is not needed to download anything from NASA servers to run the example. I know it could be misleading: I will add some comments in the code.

Concerning the new CRAN policy, however, what I will have to change for sure is the "automatic" saving of previous processing options in `ExtData/Previous/MODIStsp_previous.json". For that, I will probably ask permission to the user at first execution.

The references in vignettes/MODIStsp.pdf are not shown correctly. They contain text like "[-@hengl2010]" instead of the in-text reference.

Thanks for signalling this. The pdf vignette has been a bit neglected since I moved most information on the website. I will see that it is up-to-scratch (and also probably move it to a html version).

ROpensci packaging guidelines
This package uses RCurl. From what I can tell, the only usage of RCurl is RCurl::getURL (line 132 in R/MODIStsp_download.R) so you might be able to drop this dependency without too much hassle. This package uses base::print instead of base::message in a couple of places. This only happens in two places (lines 236 & 334 in R/MODIStsp_extract.R) so you might be able replace this with base::message fairly easily.

Agreed. I do not remember if using RCurl on that call has a reason, or it is simply a leftover of previous versions.

Review 2 - @lwasser

Installation
I ran into significant issues installing this on my mac. I still don’t have it working on my laptop but did get it working on my desktop. It would be very nice to have one fully functional set of instructions for mac that is well tested!

Agreed, and sorry for the trouble. I will try to streamline the installation instructions for mac (and to check them more thoroghly) before submitting a new version. Concerning GDAL installation, I plan to add specific instructions for mac here: http://lbusett.github.io/MODIStsp/articles/installation.html#installing-r-and-gdal

Can I ask what is the "usual" way to install GDAL with hdf4 support on a mac ? Using homebrew ? Do you think that running this would be sufficient?

brew install hdf4
brew link --overwrite hdf4
brew install gdal --complete --enable-unsupported --with-hdf4

Comments on Interface

Thanks for this very valuable and thorough feedback. Without reporting all your comments in full:

This is what I am currently planning to do:

  • Improve the Product / Layers selection interface, by adding the Product Information button also in the main GUI, and finding a way to show info about the currently selected products in the main GUI ( I have a couple of ideas in mind on that : I could share some screenshots for your comments when I implement it)

  • Add help suggestions on the "Add new index" functionality. I will however not move it in the main interface, because the Add Index functionality is product-specific and you need to be able to see which indexes are already available before adding a new one. Therefore I believe it needs to stay in the "Select Layers" area.

  • See what I can do about the "multiple boxes" issue. Consider however that that those need to be "modal" windows, so they actively block each other. Only thing I may be able to do is to have the "select layers" window disappear when the "add new window" box is opened.

  • Improve the "spatial extent" selection interface by adding helper buttons and maybe reshape a bit the interface (i.e., make the possibility to load a spatial file to specify the bounding box more preminent).

  • Modify the "reprojection selection". I think that I will leave only two options in the selection dropbox: "Native" and "User Selected" (as it happens for the resolution). This will make clear that if I use "Native" no reprojection will be done. I could also have a "warning" appear if the user specifies a different projection if you think it is needed. Consider however that MODIStsp only allows for nearest neighbour or modal resampling, so the issue concerning modifying data is reduced, IMO.

  • Improve the "processing options" (It will become "Output Format", thanks for the suggestion) and "output folder" selection sections. and provide more help. On this:

    • Concerning the saving hdfs: it allows users to specify a path (different form the "main output" one) to store the original MODIS hdf files. This is useful because:
      i) if one needs to process different areas within the same tile(s) he can download and store the HDF files only once, and use those for different processings,
      ii) It allows to create an "archive" of HDFs on a remote storage, and get only the processed results locally (for example, we keep all original HDFs on a NAS, while I can get the MODIStsp processed files on my machine),and
      iii) it allows users who already have an archive of MODIS HDFs available to process them using MODIStsp without the need of downloading again.
      I will add some help on this, and maybe move the selection widget BELOW that of the main output folder.

    • Concerning the "rasterstacks": Yes, you can create rasterStacks files automatically. This is explained in the documentation, but would probably benefit by a dedicated help button. The other option ("Create Virtual Rasters") allows instead to create virtual multitemporal files (i.e., text files pointing to the single-band images processed by MODIStsp) accessible easily from QGIS or ENVI or other image processing software. Again, I'll add some help as suggested.

    • Concerning the ".dat" files: Those are ENVI binary files. I do not remembere WHY I called them ".dat". Probably because back-in-the-day ARCGIS used ".dat" for ENVI files and I was still using it (shame on me... ;-) ) . I could change the extension to ".envi" to make things clearer. What do you think?
      As a sidenote, I tried it today, and if I select "GeoTiff" outputs, I get geotiff files in the output folder. Could you check this? Where you could have gotten ".dat" files is in the "Time_series/ENVI_META" subfolder if the "create virtual rasters" selector was set on "ENVI", or "ENVI and GDAL".

    • Concerning in general the "output conventions": The "conventions" for output storage are explained in the documentation (in particular, http://lbusett.github.io/MODIStsp/articles/output.html). Is that clear enough or do you think that additional details should be provided? (e.g., I am thinking about screenshots of a dummy folder structure, though they are ugly... ;-))

This is what I have to think a bit about:

  • Add the possibility to interactively selecting tiles on a map. On this:
  1. Consider that the current "Show Tiles Map" button does only that. That is, it shows a map to allow seeing which tiles the user should select with the sliders to get data for an area. Sorry if you wasted time "clicking" on it: nothing could have really happened ;-) . I am however surprised that ...It spun and spun and did finally load.... For me, it loads immediately. Can I ask you to tell me what happens if you run:
    library(MODIStp)
   dev.new()
   raster::plot(raster::raster(system.file("ExtData/MODIS_Tiles.gif", package = "MODIStsp")))

That is probably a "xquartz" issue that may have a workaround.

  1. Allowing to "interactively" select tiles is a good suggestion. (The MODIS package does this with the getTile function). However, I am a bit reluctant to implement it because:

    • It would require using functionalities provided by mapedit, bringing in a lot of dependencies (e.g., sf, shiny, leaflet, dplyr, mapview...)
    • It would require to open the selection "tool" in a browser or within RStudio. I personally think that one nice thing about the MODIStsp GUI is that it "packs" all the functionality in a compact, all-in-one interface.

For these reasons, I do not know if I will do something on this besides improving the GUI / help as described above. Let me know what do you think.

This is what I will not probably be able to do anything about (at least for now):

  • Improve the GUI interface for mac / XQuartz.

The MODIStsp_GUI function is very complex (and was a mess to develop...). "Reshaping" it for mac would require a lot of time and would be difficlult for me since I do not work on a mac. I am sorry if it is "clunky", but for the time being it will probably need to stay like this.

However, I may try to improve it a bit by tweaking default fonts etcetera. Would you mind sharing a couple of screenshots of the interface so that I can seem how/how much is it different from what I see on Linux or Windows (that you can look at here: http://lbusett.github.io/MODIStsp/articles/interactive_execution.html) ?

As an additional note: an obvious way to improve (?) the interface would be to go all the way down the "shiny" path. However, I am not fluent on Shiny and it would require a lot of work. Maybe on next iterations I can consider this.

Comments on Non GUI Interface / vignettes

Boy is this nice. Fully automated and reproducible downloads. Here is a bit more nitpicking that you can take or leave!

Glad you like it! Just a double check: I do not find any additional "nitpicking" here ;-) . Is something missing, or you did not have any additional comments?

Vignettes
https://cran.r-project.org/web/packages/MODIStsp/vignettes/MODIStsp.pdf
Just a note that H4 and H5 can store raster data but are not inherently raster formats

Noted.

===============

Ok, that's it. Longer than I expected but it was also useful for me to clarify the roadmap! Feel free to comment/ask for clarifications.

@karthik (when you come back)

  1. I will probably need 2-3 weeks to finalize this, since I will have to do it mostly off work hours. Is that ok for you?

  2. Can I add the "under review" badge on github?

Thanks again for all the feedback. I will keep you posted on advancements on implementation of the changes.

Lorenzo

Hi all,

a quick update: we are almost done with implementing the required modifications (You can find a commented list of work advancement here: ropensci/MODIStsp#112).

Before starting fine tuning the release (e.g., update documentation, website, etcetera) I wanted to ask (if possible) some comments on the changes we implemented on the GUI to address comments by @lwasser.

You can install the current development version using:

devtools::install_github("lbusett/MODIStsp", ref = "ropensci-review")

(Note that you will need to have the current mapedit CRAN version (0.4.1) installed for this to work - for now I added it as an import, but I plan to put it as a "Suggest" to keep the dependency tree as small as possible).

The new GUI implements most of your comments (You can see a screenshot below). In particular:

  • adds functionality for interactive selection of processing extent (In the end, I decided that it was a too good idea to skip it, and mapedit allowed me to do it relatively easily)
  • Improves selectors for time series formats
  • streamlined selection of output projection (now it is also possible to select a projection by simply specifiying an epsg code) and adds warning messages
  • adds new/improved help buttons

I still need to fine-tune it, but I wanted to ask if you think this goes in the right direction and / or if you have further comments/suggestions.

image

Thanks in advance for any feedback,

Lorenzo

Hi all,

at last, we managed to wrap-up an improved MODIStsp version addressing reviewers' suggestion. The current version can be installed with:

devtools::install_github("lbusett/MODIStsp")

In short, without going in detail through reviewers comments as in the last posts), here is a quick list of the implemented changes:

for @jeffreyhanson review:

  • All examples in vignettes and documentation now save by default in tempdir (also, at first GUI execution the output folders are set to $tempdir). If $tempdir$ is found as folder selection the outputs go to tempdir/MODStsp and tempdir/MODIStsp/HDFs

  • To avoid problems with new CRAN rules, at first MODIStsp execution the user is now asked permission to save MODIStsp previous options file in /Extdata/Previous

  • Rcurl dependency was removed. We now use httr::HEAD to check file size over ftp

  • We switched from "print" to "message" in MODIStsp_extract

  • We fixed cran warnings about "C:\Progra" etc.

  • We fixed the references in the PDF vignette

for @lwasser review:

  • We improved installation instructions for mac - fro both MODistsp package and GDAL

Improved instructions can be found at http://lbusett.github.io/MODIStsp/articles/installation.html#installing-on-mac and http://lbusett.github.io/MODIStsp/articles/installation.html#macos

Instructions were mainly derived from https://zhiyzuo.github.io/installation-rattle/. @lwasser : May you have a look and see if you think these are "good enough"?

GUI Improvements

We made various improvements in the GUI to address your comments (You can see a screenshot of the current GUI in my last post)

  • Improved the Product / Layers selection interface
  • The Product Information button is now also available in the main GUI
  • Selected layers are immediately visible in the Main Window. Help buttons were added to explain what the Quality Indicators and Spectral Indices are.
  • Added help suggestions on the "Add new index" functionality.

  • Strongly improved the "spatial extent" selection interface

  • Now it is possible to interactively select the MODIS tiles or custom spatial extent using the "Select from Map" button, and see the currently selected extent in a browser using the "Show Current Extent" button.
  • Improved / streamlined the "reprojection selection" section
  • Now only "Native" and "User Defined" can be chosen in the dropdown menu (befor, we also had some "random" projections in the dropdown because we use them often, but I think that this is cleaner)
  • When specifying a custom projection, in the Input field it is now possible to specify a projection simply by specifying an EPSG code or a UTM codes, instead of writing the full proj4 string;
  • Warning messages are now sent in case a User Projection is selected, to warn about possible problems introduced by reprojection;
  • Changing projection now re-sets the custom extent to avoid problems due to sequential reprojections
    of the extent (i.e., continuously enlarging bbox to avoid "losing" area).
  • Moved the hdf folder selector below the main output folder one and added help buttons on both output folder selectors to help the user

  • Modified the interface for selection of "formats" of virtual time series files.

  • Now we use a checkbox group allowing specifying which "time series formats" are desired, among R rasterStack, ENVI META or GDAL vrts (i.e., ENVI, R and GDAL timeseries are now "treated equally" and are selected using a single menu). We also added help buttons to facilitate the user.
  • Improved in general the help buttons and added some text formatting.

  • Changed the default output format to "GeoTiff", the default virtual raster files format to "rasterStack" and the default compression to "None"

These changes address most of the points raised by @lwasser , and I think that they in general improved user-friendliness of the interface (so, thanks for the suggestions!). The only point we were not able to address were those regarding:

When I have several boxes open (the select processing layers box and the insert new spectral index box) PLUS the original tsp interface... I have to figure out what order they are in so I can close them. Some users may think the entire thing is locked when really it’s just too many popups. This is a bit clunky for me to have 3 boxes and have to figure out the proper order! And it locked my computer up a bunch. I’m sure this is quartz specific but any way to control it?

Unfortunately, I see no easy way to do this because the different windows are "modal". I do not believe however this is a major issue. I can reevaluate if this is considered mandatory.

Other changes:

  • We improved functionality for dealing with NoData for products with multiple fill-values and fixed a couple of bugs affecting the "Seasonal" time series download (see ropensci/MODIStsp#113)

  • We made some changes in documentation. In particular, we improved some of the pkgdown articles to better help the users (see for example http://lbusett.github.io/MODIStsp/articles/noninteractive_execution.html and file:///D:/Documents/Source/git/MODIStsp/docs/articles/output.html)

  • To allow the new "interactive" spatial extent selection, we added "sf", "leaflet", "shiny", "mapview" and "mapedit" as "Suggests" in the DESCRIPTION file. If the user clicks on "Select from Map" and mapedit is not installed, he is prompted to install it. In this way, we avoid bringing in "heavy" dependencies. @karthik: do you think that is ok, or would it be better to put them in "Imports"?

OK, that's it. We hope you will like the new version and that we addressed all points raised in the review. Please let us know if you see any other issues or you need further changes/clarifications.

this all looks great @lbusett :) excited to see this package available on ropensci . it's a great contribution!!

Hi @jeffreyhanson and @karthik ,

just wanted to ask if by any chance you had time to have a look at the implemented changes.

Asking because I am planning a CRAN release to introduce the new functionality (and - more importantly - the associated bug fixes...), but I'd rather do that at the end of the review process if possible.

Hi @lbusett Will do shortly.

@karthik Ok, thanks for the update!

@jeffreyhanson Quick ping/reminder to sign off or ask more questions for @lbusett

🙏

I'm sorry for the late reply, this completely fell off my radar. @lbusett has done an amazing job and addressed all my issues/suggestions.

@jeffreyhanson @lwasser Thanks! Glad you liked the improvements! I think the review process allowed to really improve the package!

@karthik How should we proceed from here? Consider that unfortunately i will be forced to make some changes with respect to the reviewed package due to some very revcent changes in NASA policies (e.g., discontinued support for FTP download, and discontinuation of most of version 005 products). I expect to be able to release a new version by the beginning of next week.

@lbusett 👋 I'm glad to see @jeffreyhanson and @lwasser have signed off. I suggest making those changes and pushing a new release before we proceed with acceptance. I'm painfully aware of such policy changes.

@karthik

I finally managed to put out a new version (1.3.4), including changes required to address changes in NASA policies. The new version is available on github, and I plan to upload it to CRAN soon.

I'd like therefore to ask:

  1. What are the next steps for approval?
  2. Can I proceed with CRAN release or do you prefer I wait for the end of the review/approval process?

Lorenzo

@karthik, @jeffreyhanson, @lwasser

Hi all, please note that MODIStsp v 1.3.4 is now on cran. Please let me know what would be the next steps to be done for final approval on ropensci.

Thanks in advance!

Lorenzo

woo hoo! great work @lbusett ... seems like it's ready for final acceptance into ropensci by @karthik ?

Hi,

just a quick ping for @karthik, since I saw you are back to work: How can we further proceed on this?

Lorenzo

Hi @karthik

Sorry for bringing this up again, but it seems this submission got stuck in some kind of limbo, recently, and i would like to close this task. ☺️

Could we have any updates regarding next steps?

... i was just talking to someone about it and looking for this on ropensci and didn't see it and was wondering the same thing? @maelle @karthik just curious.

Sorry about the delay, i'll ping Karthik

@lbusett Apologies for the delay -- will follow up in a few.

@lbusett Sincere apologies for the delay. Your last notification came during my holiday and got lost in a giant pile of unread GitHub notifications 😢But now that I see all the Is have been dotted and T's crossed, we are ready to go.

Congrats @lbusett, your submission has been approved! 🎉 Thank you for submitting and @lwasser and @jeffreyhanson for thorough and timely reviews. To-dos:

  • Transfer the repo to the rOpenSci organization under "Settings" in your repo. I have invited you to a team that should allow you to do so. You'll be made admin once you do.

  • Add the rOpenSci footer to the bottom of your README

[![ropensci_footer](https://ropensci.org/public_images/ropensci_footer.png)](https://ropensci.org)

  • Fix any links in badges for CI and coverage to point to the ropensci URL. (We'll turn on the services on our end as needed)

Welcome aboard! We'd also love a blog post about your package, either a short-form intro to it (https://ropensci.org/technotes/) or long-form post with more narrative about its development. ((https://ropensci.org/blog/). If you are, @stefaniebutland will be in touch about content and timing.

@lbusett You can also now add the status badge (I should have alerted you earlier) to your README. It should switch to accepted shortly:

[![](https://badges.ropensci.org/184_status.svg)](https://github.com/ropensci/onboarding/issues/184)

@karthik

thanks! No worries about the delay: I know it happens. I will do those last steps ASAP.
Concerning the blog post, although I already have some blog posts regarding the package here: https://lbusett.netlify.com/categories/modis/, I think that making also a technote on rOpensci would be nice.

@lwasser @jeffreyhanson

Thanks again for your very usefeul reviews: I think they allowed me to really improve the package!
I would like to include you as reviewers in the package DECRIPTION: would that be ok with you ?

Lorenzo

Hello @lbusett. Glad to hear you're interested in contributing a technote. It might get even more eyes on your work.

Examples of technotes are here: https://ropensci.org/technotes/
Here are some technical and editorial guidelines. https://github.com/ropensci/roweb2#contributing-a-blog-post. Difference for technote is categories: technotes in YAML

We ask that you submit your draft post via pull request a week before the planned publication date so we can give you some feedback.

Happy to answer any questions.

Hi @karthik ,

I just transferred the repo to https://github.com/ropensci/MODIStsp after making the necessary changes.

(Note that the build on Travis is currently failing apparently due to a downtime of the NSIDC server)

@lbusett happy to be included as a reviewer in the description. thank you for your effort on this awesome package!! it's a great community contribution. i'm also happy to see it being embraced within the ropensci community!! yay!

@lwasser Thanks! I will add you in the description ASAP!
@jeffreyhanson What about you? Isit ok also for you to be included as a reviewer ?

@karthik

I am currently unable to modify the package settings (i.e., to modify the link to the website at the top of the repo page). Could you grant me admin settings on the repository ?

Lorenzo

Great package! I have just 1 question, while running the package I saw written that QA's were computed. In my PC several folders were created. In which folder the 'corrected' images of MODIS are? Or should I make something else (e.g. use another package) in order to take the good quality images?

@vanto1994 Please post this question in the project's repo (https://github.com/ropensci/MODIStsp/issues), not this review thread. Thanks!

@vanto1994 I took the liberty of moving the issue in the proper issues page. I will reply you there.