-
-
Notifications
You must be signed in to change notification settings - Fork 106
SITS: Satellite Image Time Series Analysis for Earth Observation Data Cubes (submit R package for review) #596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Thanks for submitting to rOpenSci, our editors and @ropensci-review-bot will reply soon. Type |
🚀 Editor check started 👋 |
Checks for sits (v1.4.1)git hash: 6eac9edf
Important: All failing checks above must be addressed prior to proceeding Package License: GPL-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. baselist (172), source (167), c (162), labels (100), nrow (72), length (62), file (55), scale (41), names (39), row (39), seq_len (38), unlist (37), col (32), date (31), unique (28), lapply (25), unname (25), q (23), sum (23), toupper (23), for (22), paste (22), return (19), as.Date (18), options (18), as.numeric (17), paste0 (17), seq_along (17), ceiling (15), class (15), apply (14), dim (14), environment (13), round (12), which (12), as.character (11), as.matrix (11), Sys.time (10), try (10), factor (9), rep (9), seq (9), sqrt (9), suppressMessages (9), url (9), array (8), do.call (8), format (8), matrix (8), ncol (8), rbind (8), sample (8), substitute (8), t (8), table (8), by (7), colnames (7), mean (7), Sys.getenv (7), tryCatch (7), character (6), data.frame (6), floor (6), formals (6), gamma (6), levels (6), strsplit (6), structure (6), version (6), which.min (6), as.integer (5), if (5), missing (5), replace (5), suppressWarnings (5), which.max (5), abs (4), any (4), as.list (4), choose (4), grepl (4), inherits (4), list2env (4), mode (4), rev (4), row.names (4), setdiff (4), tempdir (4), args (3), cbind (3), colSums (3), diag (3), eval (3), get (3), is.na (3), path.expand (3), plot (3), prop.table (3), raw (3), readRDS (3), signif (3), socketSelect (3), split (3), switch (3), system.file (3), vapply (3), as.factor (2), debug (2), deparse (2), double (2), exp (2), gsub (2), I (2), is.logical (2), log (2), logical (2), max.col (2), parent.env (2), pmin (2), quote (2), range (2), rawConnection (2), rowSums (2), sort (2), subset (2), substring (2), summary (2), tolower (2), write (2), all (1), all.vars (1), as.vector (1), basename (1), cat (1), cut (1), difftime (1), dir (1), dir.exists (1), dirname (1), drop (1), exists (1), file.exists (1), gc (1), integer (1), is.null (1), kappa (1), lengths (1), list.files (1), new.env (1), numeric (1), open (1), order (1), parent.frame (1), rbind.data.frame (1), remove (1), sample.int (1), srcfile (1), svd (1), sys.frames (1), tapply (1), tempfile (1), vector (1), within (1) utilsdata (249), txtProgressBar (3), head (2), write.csv (2), getTxtProgressBar (1), read.csv (1), tail (1) statsts (44), offset (36), formula (14), D (8), kernel (8), time (8), runif (6), predict (5), dt (4), na.action (4), quantile (4), sd (4), df (3), step (3), weights (3), as.formula (2), end (2), median (2), rbeta (2), rf (2), rlnorm (2), rnorm (2), start (2), addmargins (1), as.dendrogram (1), cutree (1), filter (1), reshape (1), smooth (1), var (1), window (1) sitssits_bands (25), sits_labels (21), sits_timeline (19), sits_select (10), train_fun (8), C_mask_na (3), mixture_fn (3), C_label_max_prob (2), C_max_sampling (2), comb_fn (2), download_fn (2), filter_call (2), label_fn (2), reclassify_fn (2), sits_train (2), sits_values (2), smooth_fn (2), .debug (1), C_entropy_probs (1), C_fill_na (1), C_kernel_max (1), C_kernel_mean (1), C_kernel_median (1), C_kernel_min (1), C_kernel_sd (1), C_least_probs (1), C_margin_probs (1), C_nnls_solver_batch (1), C_normalize_data (1), C_normalize_data_0 (1), create_iqr (1), impute_fun (1), plot_samples (1), replace_na (1), result_fun (1), seg_fun (1), sits_as_sf (1), sits_colors (1), sits_som_map (1), sits_validate (1), sits_view.sits (1), submit (1) purrrmap (63), map_chr (16), map_dfr (12), map_dfc (7), map2_dfr (7), pmap_dfr (6), map_lgl (5), pmap (4), is_character (2), map_dbl (2), imap_dfc (1), map_int (1), map2 (1), pmap_lgl (1), transpose (1) dplyrfilter (16), mutate (16), bind_rows (11), bind_cols (9), all_of (7), select (6), n (5), group_by (4), slice_sample (4), distinct (3), slice_head (3), left_join (2), select_if (2), starts_with (2), tibble (2), cur_group_id (1), inner_join (1), matches (1), summarise (1), transmute (1) ggplot2ggplot (12), aes (9), element_text (7), scale_fill_manual (5), element_rect (3), geom_line (3), labs (3), theme (3), .pt (2), element_blank (2), facet_grid (2), geom_point (2), geom_rect (2), guide_legend (2), position_dodge (2), scale_x_continuous (2), facet_wrap (1), geom_histogram (1), guides (1), scale_color_brewer (1), scale_x_date (1), scale_x_log10 (1) tibbletibble (49), as_tibble_row (10), as_tibble (7), lst (1) torchnn_module (20), torch_tensor (11), nn_cross_entropy_loss (5), nn_softmax (5), nn_batch_norm1d (4), torch_matmul (3), torch_zeros (3), nn_init_normal_ (2), torch_stack (2), torch_transpose (2), nn_dropout (1), torch_exp (1), torch_mean (1) graphicssegments (15), title (12), points (10), legend (9), plot (3), abline (2), grid (1) sfst_crs (8), st_sample (7), st_transform (7), st_as_sf (5), st_drop_geometry (3), st_intersects (3), st_coordinates (2), st_geometry (2), st_geometry_type (2), st_intersection (2), st_write (2), read_sf (1), st_bbox (1), st_convex_hull (1), st_distance (1), st_is_empty (1), st_polygon (1), st_sf (1), st_sfc (1), st_within (1) rstacpost_request (15), items_reap (11), items_fetch (9), items_matched (6), sign_planetary_computer (3), ext_query (2), stac (1), stac_search (1) terrarast (7), ext (4), spatSample (4), xFromCol (4), yFromRow (4), readValues (3), crop (2), crs (2), expanse (2), extract (2), freq (2), fileBlocksize (1), ncol (1), nlyr (1), nrow (1), readStart (1), values (1), xmax (1), xmin (1), xres (1), ymax (1), ymin (1), yres (1) sliderslide (18), slide_dfr (16), slide_dbl (4), slide_lgl (4), slide2_dfr (3), slide_chr (1), slide2_lgl (1) tidyrnest (14), unnest (11), expand_grid (9), pivot_longer (6), starts_with (2), everything (1) grDevicescolors (30), palette (9), hcl.colors (3) lubridateas_date (13), fast_strptime (1), mday (1), month (1), year (1), ymd (1) leafletlayersControlOptions (12), colorFactor (2), leaflet (1), providers$GeoportailFrance.orthos (1) digestdigest (15) tmaptm_borders (5), tm_shape (3), tm_layout (2), tm_facets (1), tm_raster (1), tmap_options (1) httradd_headers (4), write_disk (3), parse_url (2), build_url (1), content (1), GET (1) starsread_stars (8), st_warp (4) luzluz_metric_accuracy (5), setup (5), luz_callback_early_stopping (1) mgcvgam (3), predict.gam (2) yamlyaml.load_file (4), as.yaml (1) dtwclusthierarchical_control (2), cvi (1), tsclust (1) xgboostxgb.plot.tree (3), xgboost (1) caretconfusionMatrix (3) datasetstrees (3) FNNknnx.index (3) scalesdate_format (1), label_number (1), pretty_breaks (1) gdalcubesgdalcubes_options (1), image_collection (1) kohonensomgrid (1), supersom (1) methodsS3Part (2) openxlsxcreateWorkbook (1), saveWorkbook (1) e1071svm (1) exactextractrexact_extract (1) randomForestrandomForest (1) randomForestExplainerplot_min_depth_distribution (1) supercellssupercells (1) toolsfile_path_sans_ext (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 |
---|---|---|---|---|---|
5438924419 | R-CMD-check | success | bc5d6c | 116 | 2023-07-02 |
3b. goodpractice
results
R CMD check
with rcmdcheck
R CMD check generated the following note:
- checking installed package size ... NOTE
installed size is 16.7Mb
sub-directories of 1Mb or more:
libs 14.1Mb
R CMD check generated the following check_fail:
- rcmdcheck_reasonable_installed_size
Test coverage with covr
Package coverage: 0.08
The following files are not completely covered by tests:
file | coverage |
---|---|
R/api_accessors.R | 0% |
R/api_accuracy.R | 0% |
R/api_apply.R | 0% |
R/api_band.R | 0% |
R/api_bbox.R | 0% |
R/api_block.R | 0% |
R/api_check.R | 0% |
R/api_chunks.R | 0% |
R/api_classify.R | 0% |
R/api_cluster.R | 0% |
R/api_combine_predictions.R | 0% |
R/api_comp.R | 0% |
R/api_conf.R | 0% |
R/api_csv.R | 0% |
R/api_cube.R | 0% |
R/api_data.R | 0% |
R/api_debug.R | 0% |
R/api_download.R | 0% |
R/api_expressions.R | 0% |
R/api_factory.R | 0% |
R/api_file_info.R | 0% |
R/api_file.R | 0% |
R/api_gdal.R | 0% |
R/api_gdalcubes.R | 0% |
R/api_imputation.R | 0% |
R/api_jobs.R | 0% |
R/api_label_class.R | 0% |
R/api_mixture_model.R | 0% |
R/api_ml_model.R | 0% |
R/api_mosaic.R | 0% |
R/api_parallel.R | 0% |
R/api_period.R | 0% |
R/api_plot_raster.R | 0% |
R/api_plot_time_series.R | 0% |
R/api_point.R | 0% |
R/api_predictors.R | 0% |
R/api_raster_sub_image.R | 0% |
R/api_raster_terra.R | 0% |
R/api_raster.R | 0% |
R/api_reclassify.R | 0% |
R/api_roi.R | 0% |
R/api_samples.R | 0% |
R/api_segments.R | 0% |
R/api_sf.R | 0% |
R/api_shp.R | 0% |
R/api_signal.R | 0% |
R/api_smooth.R | 0% |
R/api_smote.R | 0% |
R/api_som.R | 0% |
R/api_source_aws.R | 0% |
R/api_source_bdc.R | 0% |
R/api_source_deafrica.R | 0% |
R/api_source_hls.R | 0% |
R/api_source_local.R | 0% |
R/api_source_mpc.R | 0% |
R/api_source_sdc.R | 0% |
R/api_source_stac.R | 0% |
R/api_source_usgs.R | 0% |
R/api_source.R | 0% |
R/api_space_time_operations.R | 0% |
R/api_stac.R | 0% |
R/api_stats.R | 0% |
R/api_summary.R | 0% |
R/api_tibble.R | 0% |
R/api_tile.R | 0% |
R/api_timeline.R | 0% |
R/api_torch.R | 0% |
R/api_ts.R | 0% |
R/api_tuning.R | 0% |
R/api_uncertainty.R | 0% |
R/api_utils.R | 0% |
R/api_variance.R | 0% |
R/api_view.R | 0% |
R/sits_accuracy.R | 0% |
R/sits_active_learning.R | 0% |
R/sits_apply.R | 0% |
R/sits_bands.R | 0% |
R/sits_bbox.R | 0% |
R/sits_classify.R | 0% |
R/sits_cluster.R | 0% |
R/sits_colors.R | 0% |
R/sits_combine_predictions.R | 0% |
R/sits_config.R | 0% |
R/sits_csv.R | 0% |
R/sits_cube_copy.R | 0% |
R/sits_cube.R | 0% |
R/sits_factory.R | 0% |
R/sits_filters.R | 0% |
R/sits_geo_dist.R | 0% |
R/sits_get_data.R | 0% |
R/sits_label_classification.R | 0% |
R/sits_labels.R | 0% |
R/sits_lighttae.R | 0% |
R/sits_machine_learning.R | 0% |
R/sits_merge.R | 0% |
R/sits_mixture_model.R | 0% |
R/sits_mlp.R | 0% |
R/sits_model_export.R | 0% |
R/sits_mosaic.R | 0% |
R/sits_patterns.R | 0% |
R/sits_plot.R | 0% |
R/sits_predictors.R | 0% |
R/sits_reclassify.R | 0% |
R/sits_regularize.R | 0% |
R/sits_resnet.R | 0% |
R/sits_sample_functions.R | 0% |
R/sits_segmentation.R | 0% |
R/sits_select.R | 0% |
R/sits_sf.R | 0% |
R/sits_smooth.R | 0% |
R/sits_som.R | 0% |
R/sits_summary.R | 0% |
R/sits_tae.R | 0% |
R/sits_tempcnn.R | 0% |
R/sits_temporal_segmentation.R | 0% |
R/sits_timeline.R | 0% |
R/sits_train.R | 0% |
R/sits_tuning.R | 0% |
R/sits_uncertainty.R | 0% |
R/sits_utils.R | 12.5% |
R/sits_validate.R | 0% |
R/sits_values.R | 0% |
R/sits_variance.R | 0% |
R/sits_view.R | 0% |
R/sits_xlsx.R | 0% |
src/combine_data.cpp | 0% |
src/kernel.cpp | 0% |
src/label_class.cpp | 0% |
src/linear_interp.cpp | 0% |
src/nnls_solver.cpp | 0% |
src/normalize_data_0.cpp | 0% |
src/normalize_data.cpp | 0% |
src/sampling_window.cpp | 0% |
src/smooth_bayes.cpp | 0% |
src/smooth_sgp.cpp | 0% |
src/smooth_whit.cpp | 0% |
src/smooth.cpp | 0% |
src/uncertainty.cpp | 0% |
Cyclocomplexity with cyclocomp
The following function have cyclocomplexity >= 15:
function | cyclocomplexity |
---|---|
sits_cube.stac_cube | 16 |
Static code analyses with lintr
lintr found the following 23 potential issues:
message | number of times |
---|---|
Avoid library() and require() calls in packages | 16 |
Lines should not be more than 80 characters. | 3 |
Use <-, not =, for assignment. | 4 |
Package Versions
package | version |
---|---|
pkgstats | 0.1.3.4 |
pkgcheck | 0.1.1.26 |
Editor-in-Chief Instructions:
Processing may not proceed until the items marked with ✖️ have been resolved.
Many thanks for your response. Please see below the following explanation, which was included as an "Important Note" in the submission, but maybe it has failed to catch the attention of the reviewers.
✖️ The following function has no documented return value: [sits_filter]
✖️ Package has no HTML vignettes
✖️ Package coverage is 0.1% (should be at least 75%).
We are confident that We would also like to respond to the
The package imports directly 17 packages, which are required for most functions. It also suggests 33 packages, which are typically used only in a few function, and need to be included only in "as-is" basis. This is based on CRAN policies that restrict the number of imported packages. |
Thank you for your submission @gilbertocamara! As well as your careful response to the automatic checks. We agree with all your responses above. However, as this a package that implements statistical and ML methods of geospatial data, rather than just the “accessing, manipulating, converting” and converting in our scope, it falls under under our newer statistical peer review program, which has its own time series and [geospatial standards](https://stats-devguide.ropensci.org/standards.html#standards-spatial. Submission requirements are different for this as authors need to document their standards compliance with our code annotation system. A note - SITS is a package that is very large in scope and code base, as exemplified by the fact that it has a whole book for its documentation. As such, we anticipate that it will be challenging to find reviewers and we will need to give them considerably longer than usual to review the code base and documentation in full. Most of our submissions are not as large or mature at the point of review and up for significant API or architecture changes in response to review. For something in an earlier stage we would likely have suggested breaking functionality up into smaller, more focused packages. Nonetheless, we are up for the challenge if you are up for the higher statistical submission requirements and potential changes. One last note regarding check results: As of now we can't set any environment variables when running the checks automatically, so they'd have to be set on your side, maybe using the withr::local_envvar() function or similar. Thanks again! We're happy to answer further questions. |
Dear @maelle, many thanks for your response. Please see my comments below:
Good! Looking at the specific requirements for ROpenSci statistical packages, the At a first glance,
Allow me to propose an alternative: please consider that the information provided in "codecov.io" to be sufficient to assert that
We will work on improving Thanks, |
Thank you! 🎉 Here's a direct link to the author guide for stat submissions: https://stats-devguide.ropensci.org/pkgdev.html |
@gilbertocamara just a clarification: your package will have to comply with one of the category standards of the statistical review system, probably spatial (because time series is for class-based manipulation of time-series data, which is not what your package does as far as I understand). Probability distributions should be considered an "additional' category that may be complied with in addition to the main categories. Thank you! |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
@gilbertocamara the CONTRIBUTING guide could link to the book chapter, as long as it's easy to find all information. I'm still waiting for R CMD check to finish, but examples ran without error (tests now running). |
Thanks for the tips! |
Tests passed! Now on to trying autotest... |
Dear @maelle, autotest runs OK in "sits". Now, we have to work on the recommendations. Thanks! |
Dear @maelle @mpadge I would like to ask for your help to understand how In the Consider the following function, which takes as input a set of spatially referenced time series and allows the user to select some of its members. Users can either select a number or a fraction of the series. The relevant part of the code is shown below:
The output for autotest for this function is:
In the above the
We are failing to understand what is being tested by
We are failing to see what we might be doing wrong. What are the expectations of We would appreciate your response. Best |
Please, could you explain what appears to be an unexpected behaviour of Today, I ran I tried to fix some of these problems by considering the recommendations of the tidyverse design guide. In Section 26 ("Side-effect functions should return invisibly"), the guide states: "If a function is called primarily for its side-effects, it should invisibly return a useful output. If there’s no obvious output, return the first argument". See more at https://design.tidyverse.org/out-invisible.html. I am assuming that Could you please help me and explain why *** MWE ***
Best regards |
Hello! I'll get to this later this week, thanks for your patience! |
Dear @maelle @mpadge Begging your indulgence for being insistent, I would like to ask if there is a detailed explanation of the types of diagnostics provided by Out of these 18 instances, Since the condition to proceed with the revision for statistical packages submitted to ROpenSci is that Please also explain why Many thanks for your help, |
res <- autotest::autotest_package("/home/maelle/Documents/ropensci/SOFTWARE-REVIEW/sits", test = TRUE)
#> Loading required namespace: devtools
#> ℹ Loading sits
#> SITS - satellite image time series analysis.
#>
#> Loaded sits v1.4.2.
#> See ?sits for help, citation("sits") for use in publication.
#> Documentation avaliable in https://e-sensing.github.io/sitsbook/.
#>
#> ★ Extracting example code from 107 .Rd files
#> | | | 0% | |= | 1% | |= | 2% | |== | 3% | |=== | 4% | |=== | 5% | |==== | 6% | |===== | 7% | |====== | 8% | |======= | 9% | |======= | 10% | |======== | 11% | |========= | 12% | |========= | 13% | |========== | 14% | |========== | 15% | |=========== | 16% | |============ | 17% | |============ | 18% | |============= | 19% | |============== | 20% | |============== | 21% | |=============== | 21% | |================ | 22% | |================ | 23% | |================= | 24% | |================== | 25% | |================== | 26% | |=================== | 27% | |==================== | 28% | |==================== | 29% | |===================== | 30% | |====================== | 31% | |====================== | 32% | |======================= | 33% | |======================== | 34% | |======================== | 35% | |========================= | 36% | |========================== | 36% | |========================== | 37% | |=========================== | 38% | |=========================== | 39% | |============================ | 40% | |============================= | 41% | |============================= | 42% | |============================== | 43% | |=============================== | 44% | |=============================== | 45% | |================================ | 46% | |================================= | 47% | |================================= | 48% | |================================== | 49% | |=================================== | 50% | |==================================== | 51% | |===================================== | 52% | |===================================== | 53% | |====================================== | 54% | |======================================= | 55% | |======================================= | 56% | |======================================== | 57% | |========================================= | 58% | |========================================= | 59% | |========================================== | 60% | |=========================================== | 61% | |=========================================== | 62% | |============================================ | 63% | |============================================ | 64% | |============================================= | 64% | |============================================== | 65% | |============================================== | 66% | |=============================================== | 67% | |================================================ | 68% | |================================================ | 69% | |================================================= | 70% | |================================================== | 71% | |================================================== | 72% | |=================================================== | 73% | |==================================================== | 74% | |==================================================== | 75% | |===================================================== | 76% | |====================================================== | 77% | |====================================================== | 78% | |======================================================= | 79% | |======================================================== | 79% | |======================================================== | 80% | |========================================================= | 81% | |========================================================== | 82% | |========================================================== | 83% | |=========================================================== | 84% | |============================================================ | 85% | |============================================================ | 86% | |============================================================= | 87% | |============================================================= | 88% | |============================================================== | 89% | |=============================================================== | 90% | |=============================================================== | 91% | |================================================================ | 92% | |================================================================= | 93% | |================================================================== | 94% | |=================================================================== | 95% | |=================================================================== | 96% | |==================================================================== | 97% | |===================================================================== | 98% | |===================================================================== | 99% | |======================================================================| 100%
#> ✔ Extracted example code
#> ★ Converting 47 examples to yaml
#> | | | 0% | |= | 2% | |=== | 4% | |==== | 6% | |====== | 9% | |======= | 11% | |========= | 13% | |========== | 15% | |============ | 17% | |============= | 19% | |=============== | 21% | |================ | 23% | |================== | 26% | |=================== | 28% | |===================== | 30% | |====================== | 32% | |======================== | 34% | |========================= | 36% | |=========================== | 38% | |============================ | 40% | |============================== | 43% | |=============================== | 45% | |================================= | 47% | |================================== | 49% | |==================================== | 51% | |===================================== | 53% | |======================================= | 55% | |======================================== | 57% | |========================================== | 60% | |=========================================== | 62% | |============================================= | 64% | |============================================== | 66% | |================================================ | 68% | |================================================= | 70% | |=================================================== | 72% | |==================================================== | 74% | |====================================================== | 77% | |======================================================= | 79% | |========================================================= | 81% | |========================================================== | 83% | |============================================================ | 85% | |============================================================= | 87% | |=============================================================== | 89% | |================================================================ | 91% | |================================================================== | 94% | |=================================================================== | 96% | |===================================================================== | 98% | |======================================================================| 100%
#> ✔ Converted examples to yaml
#>
#> ── autotesting sits ──
#>
#> ✔ [1 / 19]: sits_clean
#> ✔ [2 / 19]: sits_cluster_clean
Created on 2023-07-18 with reprex v2.0.2 |
@gilbertocamara regarding the output you mentioned in your comment #596 (comment), can you confirm it's gone? I don't see that exact error (I'm going through your comments chronologically). |
Dear @maelle, above you have shown the |
Good question. To me the best answer is currently https://docs.ropensci.org/autotest/reference/autotest_types.html, does it help? I opened an issue in autotest because I agree the documentation could be improved on this front ropensci-review-tools/autotest#83 |
currently actually running the tests 😅 sorry about that |
Regarding the flagging of 2/18 functions, obviously I'll have a better idea once I have the results locally, but since autotest works by scraping examples, this might be due to different examples in these 2 functions? |
I updated the results I get. Are they the same as on your machine @gilbertocamara? |
Unfortunately, no. Please wait a little bit. Yesterday, we made some changes to |
Dear @mpadge, @maelle, @Nowosad, @alexgleith, @mdsumner, @edzer, @loreabad6, this is an update on the improvements requested by the reviewers: (a) Improve function documentation: major update of most sits functions, including (b) Provide conversion of sits objects to/from other R packages: conversion from (c) Quoting in 'sits_labels<-' replacement function: review done, we prefferred to keep the current version. (d) Improving detection of snow cover: done and verified by @loreabad6. (e)Improve plot: requested by @Nowosad, completed, waiting for verification by @Nowosad. (f) Improve creating local cubes: major update on the documenation of We consider that the requested changes in the code and in the function documentation have been mostly addressed. We are now going to focus on improving the online book, as requested. Many thanks for your excellent contribution, |
📆 @mdsumner you have 2 days left before the due date for your review (2025-04-14). |
📆 @loreabad6 you have 2 days left before the due date for your review (2025-04-14). |
📆 @Nowosad you have 2 days left before the due date for your review (2025-04-14). |
📆 @edzer you have 2 days left before the due date for your review (2025-04-14). |
📆 @alexgleith you have 2 days left before the due date for your review (2025-04-14). |
Dear @mpadge @maelle Please allow @loreabad6 @Nowosad @alexgleith @mdsumner @edzer at least until the end of May to complete their reviews. The comments by reviewers have been as helpful as they are demanding. Their remarks can be broken into two partes: (a) package code and function documentation; (b) important changes in the online book. We have almost completed part (A) and will be starting on part (B) next week. It will take us about a month to make all the requested changes in the online book. So, please, allow more time by the reviewers to be able to complete their work. |
I haven't been able to get to the examples that Lorena provide me but will try to very soon, I also wanted to meet to see how the process looked but I couldn't in the end because of timetabling (sorry @loreabad6), we might still be able to do that and I also asked to sign up myself so I may get to have a closer look. I will also ask a colleague who I think has access to Planet. I wanted to discuss the GDAL topic with the authors, my poorly expressed and interrupted "one thing", but frankly I consider that out of scope here. The efforts of the sits authors within a very challenging software ecosystem are absolutely stellar and while I hope to get to more considered feedback again I don't see any roadblocks at all, the package is an excellent body of work and has strived really hard to make the best of the available software and plumbing required. This scene is also ever evolving, STAC being born in "always traverse the tree" json is now starting to come down to tidier tabular principles with GeoParquet, GTI, and sits itself was a strong early iilustration in treating the "cube" as a table of related but as yet uneresolved inputs that are always unhidden and available to user at any level. I think it's excellent. (Yet more challenges coming very quickly with Sentinel-in-Zarr, but GDAL is keeping up there and any discussion of those changes that impact R should occur there and in (frankly mostly Python-!) Zarr itself). For preparation to the committee meeting, and in subsequent light reading and exploring I spent 8.5 hours reviewing. I will happily spend more time if the duration is extended, but I consider the feedback and response so far to be sufficient for this purpose. |
Dear @mdsumner, your support and your help are greatly appreciated. For your reference and that of @Nowosad, @alexgleith, @edzer, @loreabad6, @mpadge, please allow me to try to explain how we use GDAL in
Every six months, @rolfsimoes, @OldLipe, @M3nin0 and me discuss replacing Another key point is that, by relying on Thus, long story short, all geospatial R developers rely (whether they know it or not) on @edzer to carry the heavy load. I hope this clarifies the views of the |
Dear @mdsumner, your generous comments are much appreciated. There are two complementary viewpoints on STAC. From a conceptual Data Science point of view, STAC is a step in reverse gear. Before STAC came to prominence, @alexgleith and his team at DE Australia had shown there was a better solution based on well-established object-relational database structures. Had the solution originally conceived by @alexgleith and DE Australia prevailed, we would have a much better mechanism for setting up queries to cloud providers. The success of RDBMS in commercial markets shows that the ODC original solution was the best option. STAC is the VHS of the big EO data world (or should we say Windows-95?), Despite our grievances, STAC is here to stay; even ODC had to adapt to this reality. From a Computer Science point of view, STAC smells of the 1970s. Nevertheless, there are many 1970s technologies still very much alive (think C, UNIX, and LaTeX). My CS side regrets that we have been left with STAC. At the end of the day, clever people such as the STAC lead developers can do good things even with questionable specifications. BTW, I am a proud user of LaTeX and have no plans to change anytime soon. Reality always trumps utopias. So our team of @rolfsimoes, @OldLipe and @M3nin0 also had to develop |
@ropensci-review-bot submit review #596 (comment) time 3 |
Logged review for mdsumner (hours: 3) |
@ropensci-review-bot submit review #596 (comment) time 3 |
Logged review for loreabad6 (hours: 3) |
@ropensci-review-bot submit review #596 (comment) time 5 |
Logged review for Nowosad (hours: 5) |
@edzer @alexgleith A repeat of previous request: Could you both make brief statements here like this and this that you were part of the review committee, and support the review posted by @mdsumner. Thank you |
As part of the review committee, I also support the above review comments, as well as the summary of the review discussion posted by @mdsumner . |
@ropensci-review-bot submit review #596 (comment) time 3 |
Logged review for edzer (hours: 3) |
For information of @edzer @loreabad6 @alexgleith @Nowosad @mdsumner @mpadge, I believe we have completed all issues related to coding of Although not specifically requested by you, we also performed a significant code revision, based on guidance from
We also revised all checking functions involved in preconditions. Thanks for making us do a lot of work now, which has improved the software! for reference, copied to @rolfsimoes @M3nin0 @OldLipe |
@gilbertocamara Do you mean that you've now finished all responses to review comments? Or do you still intend to implement further changes? Please let us know when you have completely finished, at which time it would also be helpful if you could provide a summary of all changes implemented, ideally with links to where everybody can see your improvements. Thanks for responding so thoroughly, and so quickly! |
Hi @mpadge, the reviewers made two types of requests: (a) improvements on the package code and function documentation; (b) Improvements on the online book. Thus we created two sets of issues: As a part of this process, the following issues have been opened:
#1306 quoting in 'sits_labels<-' replacement function - ADDRESSED #1307: sits_regularize: "named" issue - ADDRESSED #1308 Improving detection of snow cover - SOLVED #1309 Improve function documentation - SOLVED #1310 Improve and review plot - SOLVED #1311 Improve creation of data cubes from PlanetScope data - ADDRESSED #1313 Provide conversion of sits objects to/from other R packages - SOLVED #1322 Revise sits code to harmonize API usage and conform to best practices and lintr - SOLVED
#1312 Improve documentation of configuration files #1314 Improve the documentation by include an Overview chapter with concise, objective use cases Since we consider to have addressed and solved all coding-related issue, we will now concentrate on the online book. I would like to ask @mpadge @loreabad6 @edzer @Nowosad @mdsumner @alexgleith to signal if they disagree with our decision. Otherwise we will concentrate on the book. We will keep you informed on progress on the book. Best regards |
Sounds great @gilbertocamara, please continue then with the book updates, and let us know here when you're done with those. Thanks as always for the very quick and informative response. |
@gilbertocamara just to thank you for sharing your work with rOpenSci. I'm wrapping up my EiC rotation today. I look forward to working with you again. |
Dear @mpadge @loreabad6 @edzer @Nowosad @mdsumner @alexgleith, this message is to update you on the status of required improvements on |
That's great news, thanks for the update @gilbertocamara. We'll look forward to news of further updates in around a month's time. |
Uh oh!
There was an error while loading. Please reload this page.
Submitting Author Name: Gilberto Camara
Due date for @Nowosad: 2025-04-14Submitting Author Github Handle: @gilbertocamara
Other Package Authors Github handles: @rolfsimoes, @OldLipe, @M3nin0, @lorenalves
Repository: https://github.com/e-sensing/sits
Version submitted: 1.4.2
Submission type: Standard
Editor: @mpadge
Reviewers: @mdsumner, @loreabad6, @Nowosad, @edzer, @alexgleith
Due date for @alexgleith: 2025-04-14
Archive: TBD
Version accepted: TBD
Language: en
Scope
Please indicate which category or categories from our package fit policies this package falls under:
Explain how and why the package falls under these categories (briefly, 1-2 sentences):
sits
is a package for satellite image time series analysis that works with big Earth observation data sets.Who is the target audience, and what are the scientific applications of this package?
Remote sensing and environmental experts wanting to classify remote sensing images for applications such as deforestation detection, agricultural and land use/land cover mapping, biodiversity conservation, and land degradation monitoring comprise the target audience.
Are there other R packages that accomplish the same thing? If so, how does yours differ or meet our criteria for best-in-category?
There are currently no other open-source software packages that have the same capabilities.
(If applicable) Does your package comply with our guidance around Ethics, Data Privacy and Human Subjects Research?
Not applicable
If you made a pre-submission inquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted.
Not applicable
Explain reasons for any
pkgcheck
items which your package is unable to pass.(a) Vignettes: Instead of preparing vignettes, the authors have written an online book that describes the contents of the package in detail. The book is available at the URL https://e-sensing.github.io/sitsbook/
Important notes:
(1) To run the tests, examples, and code coverage, please make
Ensure that the following environment variables are set in the R session.
Sys.setenv("SITS_RUN_TESTS" = "YES")
Sys.setenv("SITS_RUN_EXAMPLES" = "YES")
sits
is a fairly large package, and the tests take a long time to run, since they access cloud services. We must manually enable testing for this reason.(2) Please review version 1.5.3, not yet on CRAN, which is available in the "dev" branch in the GitHub repository.
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?
The package is already on CRAN.
Do you intend for this package to go on Bioconductor?
[ x] 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: