rsbivand / rgrass

Interpreted interface between the GRASS geographical information system and R
https://rsbivand.github.io/rgrass/
24 stars 8 forks source link

Rast ng probably ready to merge #45

Closed rsbivand closed 2 years ago

rsbivand commented 2 years ago

The rastNG branch provides re-implementations using the "SpatRaster" and "SpatVector" classes from terra, and using terra methods for reading and writing intermediate files (See #42). The additions need checking on (R CMD check rgrass7_0.2-8.tar.gz in nc_basic_spm_grass7 location): rgrass7_0.2-8.tar.gz or this example file: rgrass7-Ex.zip

before merging (possibly other Linux versions if contributions are possible).

rsbivand commented 2 years ago

Yes, the path I'm suggesting is to deprecate read/write/RAST/VECT without underscore soon, and only use terra leaving r.in/out.bin for sp raster. Coercion to other representation formats would then be up to the user. That will be easier to maintain, I think, but welcome discussion. If for example stars is a better fit for temporal multiband rasters, that could be added later.

rsbivand commented 2 years ago

@VLucet and @florisvdh Since no negative review, I'll merge tomorrow to rgrass7 and rgrass, and rgrass7 to main shortly thereafter, unless I hear why this should not be done.

florisvdh commented 2 years ago

@rsbivand sorry, I had missed your review request. I can look at it later today.

rsbivand commented 2 years ago

@florisvdh I don't think that enabling the initGRASS() examples when running R in an active GRASS environment is sensible, because GRASS is already running. There is no way to test initGRASS() (GRASS in R) and regular (R in GRASS) at the same time, I think.

florisvdh commented 2 years ago

There is no way to test initGRASS() (GRASS in R) and regular (R in GRASS) at the same time, I think.

Could be, depending on the circumstances, although I didn't run into problems so far. Rerunning initGRASS() in a R-in-GRASS session just resets the needed env variables and seems to work well, using the same GRASS version, although I wouldn't try/recommend to call another GRASS version that way. The examples at the bottom of https://github.com/rsbivand/rgrass/pull/45#pullrequestreview-895402640 were run as R within GRASS and also contain a change of the location. Rerunning some lines, from R within GRASS 8.0.1 (actually RStudio, see GRASS session code):

Invoking RStudio from GRASS (click to expand) ```bash $ /usr/local/grass8.0.1/bin/grass Starting GRASS GIS... Cleaning up temporary files... __________ ___ __________ _______________ / ____/ __ \/ | / ___/ ___/ / ____/ _/ ___/ / / __/ /_/ / /| | \__ \\_ \ / / __ / / \__ \ / /_/ / _, _/ ___ |___/ /__/ / / /_/ // / ___/ / \____/_/ |_/_/ |_/____/____/ \____/___//____/ Welcome to GRASS GIS 8.0.1 GRASS GIS homepage: https://grass.osgeo.org This version running through: Bash Shell (/bin/bash) Help is available with the command: g.manual -i See the licence terms with: g.version -c See citation options with: g.version -x Start the GUI with: g.gui wxpython When ready to quit enter: exit To run a command as administrator (user "root"), use "sudo ". See "man sudo_root" for details. GRASS nc_basic_spm_grass7/PERMANENT:git_repositories > rstudio 2> /dev/null & [1] 6947 GRASS nc_basic_spm_grass7/PERMANENT:git_repositories > ```
R code and output (click to expand) ```r > library(rgrass7) Loading required package: XML GRASS GIS interface loaded with GRASS version: GRASS 8.0.1 (2022) and location: nc_basic_spm_grass7 > execGRASS("g.version") GRASS 8.0.1 (2022) > execGRASS("g.gisenv") GISDBASE='/home/floris/grassdata'; LOCATION_NAME='nc_basic_spm_grass7'; MAPSET='PERMANENT'; GUI='text'; PID='6927'; > > > setwd("~/ContinuLegen/rgrass_test/") > getwd() [1] "/media/floris/DATA/ContinuLegen/rgrass_test" > my_GRASS <- "/usr/local/grass8.0.1/grass80" > loc <- initGRASS(my_GRASS, home=tempdir(), gisDbase=".", + location="nc_basic_spm_grass7", mapset="user1", override=TRUE) > execGRASS("g.version") GRASS 8.0.1 (2022) > execGRASS("g.gisenv") GISDBASE='.'; LOCATION_NAME='nc_basic_spm_grass7'; MAPSET='user1'; GRASS_GUI='text'; > > > library(terra) terra 1.5.12 > f <- system.file("ex/elev.tif", package="terra") > r <- rast(f) > loc <- initGRASS(my_GRASS, home=tempdir(), SG=r, override=TRUE) > execGRASS("g.gisenv") GISDBASE='/tmp/grass8-floris-6922/RtmpvZDi8a'; LOCATION_NAME='file1b615d871075'; MAPSET='file1b616c223a0d'; GRASS_GUI='text'; ```
rsbivand commented 2 years ago

I think I'd prefer to write a short vignette rather than use examples to demonstrate the different wayss of using the package, as a new PR. I just marked the old functions as deprecated, which may alert users to forthcoming changes. Are we OK to merge?

florisvdh commented 2 years ago

Yes, a vignette is the best way.

I just marked the old functions as deprecated

Not pushed that commit yet?

Are we OK to merge?

I didn't encounter problems and totally trust you!

rsbivand commented 2 years ago

Thanks, now pushed! And that's why trusting totally implies trusting collaborative processes!

rsbivand commented 2 years ago

Thanks, I updated the other deprecation messages.

florisvdh commented 2 years ago

Thanks, all looks good!