ropensci / statistical-software-review-book

Guide for development and peer-review of statistical software
https://stats-devguide.ropensci.org
43 stars 11 forks source link

Spatial standards - review by JS #30

Open jsta opened 3 years ago

jsta commented 3 years ago

General comments

Overall this looks like a great section of the book! Happy to submit a PR (or two) if I get a thumbs up.

Things I felt were missing

  1. I think all software should either work with XYZ dimension objects or check for a Z dimension and raise a specific error.
  2. I wonder if some standard related to netCDF should be implemented. I don't have anything specific in mind but surely there are ways to go wrong handling it. It would be great if all netCDF packages did something (without a cryptic error) given only a file path and nothing else (i.e. no fore-knowledge of layer structure or indexing).

Specific comments




mpadge commented 3 years ago

Thanks a lot @jsta for these very useful comments. It would be great if you could transform them into direct edits and submit a pull request, especially as direct contributions to the primary document are something we are very actively wanting to encourage and cultivate. A few responses from my side to help ensure we're seeing things the same way, beginning with the following comments of yours with which i entirely agree and ask you to formulate something specific via pull request:

  1. References to Z dimension
  2. EPSG codes as alternative to PROJ4 strings
  3. Your comment about not necessarily accepting "input data is as many specific spatial classes as possible," but "picking one and supporting it well" is a good point, in line with which we could either delete that standard entirely (because the principle is sufficiently covered in the preceding standards anyway), or you could maybe re-formulate it to say something more useful?
  4. Your comment about the standard on using the units package: by "not using" is meant that a package need not Import, Depend or directly use routines from the units package, but merely that input data with components in classes defined by this package should be accepted. Any clarifications of that also appreciated!

Responses to other comments:

  1. Your comment about netCDF is perhaps a bit too specific. That's only likely to be relevant to a very small portion of the spatial community in general, and so better handled directly somewhere like the tidync package.
  2. Your discovery of that broken link has been rectified; thanks!

Looking forward to some more direct input from you - please feel free to continue the discussion there. Thank you! (You can also close this issue once you've - hopefully! - proceeded on over to a pull request.)