Skip to content
This repository has been archived by the owner on Apr 20, 2020. It is now read-only.

Problem with RQGIS not closing the file connection with QGIS #82

Closed
BastienFR opened this issue Sep 28, 2017 · 23 comments
Closed

Problem with RQGIS not closing the file connection with QGIS #82

BastienFR opened this issue Sep 28, 2017 · 23 comments

Comments

@BastienFR
Copy link

BastienFR commented Sep 28, 2017

Hello,

I've found a problem with the package RQGIS that is preventing me from having a good workflow. The problem is that (I think) the package doesn't close the file connection with QGIS after the analysis. Here is a reproducible example:

You can download my raster here: https://www.dropbox.com/s/6zsmz5kxp0d86tu/depth_D500.tif?dl=0 but I'm pretty sure any raster will have the problem.


library(RQGIS)
monqgis <- set_env("C:\\OSGeo4W64")

params.default.rastercalculator <- get_args_man(alg = "gdalogr:rastercalculator", qgis_env = monqgis)
params <- params.default.rastercalculator
path <- "C:\\Users\\Bastien\\Dropbox\\stackexchange_tranfert\\_temp\\"

params$INPUT_A <- paste0(path,"depth_D500.tif")
params$OUTPUT <- paste0(path, "depth_D500b.tif")
params$FORMULA <- "A*2" 
params$RTYPE <- "1"
system.time(run_qgis(alg = "gdalogr:rastercalculator", params=params, qgis_env = monqgis))

file.remove(list.files(path, full.names = T))
[1] FALSE  TRUE  TRUE
Warning message:
In file.remove(list.files(path, full.names = T)) :
  cannot remove file 'C:\Users\Bastien\Dropbox\stackexchange_tranfert\_temp\depth_D500.tif', reason 'Permission denied'

As you can see, when I'm trying to delete my files, I cannot delete one of them (in that case depth_D500.tif) which is the input of my analysis. The only way I can delete it after is by closing R. Then the connection is drop and I can delete it. This is annoying because I want to delete my temporary files to reduce disk usage.

I think the problem is generalized (i.e. not limited to the "gdalogr:rastercalculator" module) as I experienced it with "grass7:r.reclass" as well.

my session info:

R version 3.4.1 (2017-06-30)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows Server >= 2012 x64 (build 9200)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.1252  LC_CTYPE=English_United States.1252    LC_MONETARY=English_United States.1252
[4] LC_NUMERIC=C                           LC_TIME=English_United States.1252    

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

other attached packages:
[1] RQGIS_1.0.1.9000 reticulate_1.0  

loaded via a namespace (and not attached):
 [1] Rcpp_0.12.12    lattice_0.20-35 sf_0.5-4        grid_3.4.1      R6_2.2.2        DBI_0.7         jsonlite_1.5    magrittr_1.5    units_0.4-6    
[10] stringi_1.1.5   rlang_0.1.1     raster_2.5-8    sp_1.2-5        rgdal_1.2-11    tools_3.4.1     stringr_1.2.0   udunits2_0.13   readr_1.1.1    
[19] hms_0.3         parallel_3.4.1  compiler_3.4.1  tibble_1.3.3   

Thanks
Bastien

@jannes-m
Copy link
Collaborator

Probably there's nothing we can do here.We asked the reticulate developers if there was a way to manually close the Python tunnel, they were afraid this would cause too much trouble.

@jannes-m
Copy link
Collaborator

Probably there's nothing we can do here.We asked the reticulate developers if there was a way to manually close the Python tunnel, they were afraid this would cause too much trouble (see .rstudio/reticulate#27). Will have a look when I am back from my holidays.

@BastienFR
Copy link
Author

BastienFR commented Sep 29, 2017

That is an horrible news! I've integrated RQGIS into my work process which do multiple task on raster tiles and save temporary files in the process. I was hoping to parallelized or loop through all my tiles. For that I needed to delete the temporary files as they become useless. If I don't do it, I'll finish with Terabytes of useless data and it won't work... I don't know how I'll get around it... I may need to use directly gdalUtils and rGRASS but I didn't wanted to, preferring to stick with RQGIS. I'll think about it.
Thanks for your time.

@jannes-m
Copy link
Collaborator

Sorry to hear that.Maybe there is a way to close a custom QGIS Python application. There soil should be, however, there me nasty side effects. As I said, will look into it when I am back.

@jannes-m
Copy link
Collaborator

I just ran:

library("RQGIS")
run_qgis(alg = "gdalogr:rastercalculator", 
                     INPUT_A = dem,
                     OUTPUT = "depth_D500b.tif",
                     FORMULA = "A*2",
                     RTYPE = 1)

file.remove(file.path(tempdir(), "depth_D500b.tif"))

This worked for me, i.e. the output file was removed. Therefore, I don't think that your problem is related to (R)QGIS but rather to your access rights (but just guessing). Try also to run RStudio or R as an administrator.

@jannes-m
Copy link
Collaborator

sorry, you were referring to the input file, ok, will have a look.

@jannes-m
Copy link
Collaborator

py_run_string("app.exitQgis()")

lets you close the QGIS session, and then you can also delete the input file, in your case this was depth_D500.tif. However, then we have to reinitialize the QGIS session, and I have to take a look if and how this can be done. But for now, I have to go, I will have a look when I am back (10.10.), so be a bit patient.

@BastienFR
Copy link
Author

That's already a big help, thanks and have a good vacation! I'll wait!

@jannes-m
Copy link
Collaborator

jannes-m commented Oct 2, 2017

Have been doing some digging, and it seems you have encountered a QGIS problem. Apparently, QGIS locks the input files. You can release the lock, though this is a bit more complicated than I have previously suspected (see e.g. this post and this one).

The bad new is that fiddling around with the QGIS session - our initial approach (QgsApplication.exitQgis()) - and restarting it, will most certainly crash Python, QGIS and/or R.

The good news is, there might be a way to solve this issue

Taking your example as a use case again:

# save raster to a file
file = normalizePath(file.path(tempdir(), "dem.tif"), "/")
writeRaster(dem, file)
# import raster into QGIS and register it
py_run_string(sprintf("rlayer = QgsRasterLayer('%s', 'input')", file))
py_run_string("QgsMapLayerRegistry.instance().addMapLayer(rlayer)")
# run the processing
py_run_string(
  paste("processing.runalg('gdalogr:rastercalculator', {'INPUT_A':rlayer",
        "'OUTPUT':'C:/Users/pi37pat/AppData/Local/Temp/RtmpWmA1KZ/depth.tif'",
        "'FORMULA':'A*2',  'RTYPE':1})", sep = ","))
# remove the input raster layer again
py_run_string("QgsMapLayerRegistry.instance().removeMapLayer(rlayer.id())")
py_run_string("del rlayer")
# delete it from disk
file.remove(file)

However, this approach requires some tweaking of the existing code base, and I have to think about possible side effects. Will put the new code into a new branch but again give me some time.

@jannes-m
Copy link
Collaborator

jannes-m commented Oct 3, 2017

I have created a new branch called unlock. You can install it using:

devtools::install_github("jannes-m/RQGIS", ref = "unlock")

This should address your issue:

data("dem", package = "RQGIS")
raster::writeRaster(dem, file.path(tempdir(), "dem.tif"))
run_qgis(alg = "gdalogr:rastercalculator", 
                     INPUT_A = file.path(tempdir(), "dem.tif"),
                     OUTPUT = "depth_D500b.tif",
                     FORMULA = "A*2",
                     RTYPE = 1)
file.remove(list.files(tempdir(), full.names = TRUE))

Let me know, if this works for you or if you encounter any trouble. Please note that the work on this branch is not entirely done. I have to still find a way how to deal with multipleinput parameters.

@jannes-m jannes-m self-assigned this Oct 3, 2017
@BastienFR
Copy link
Author

BastienFR commented Oct 4, 2017

I've tried this code (which is my real code) and it works perfectly:

library(RQGIS);monqgis <- set_env("C:\\OSGeo4W64")
params.default.rastercalculator <- get_args_man(alg = "gdalogr:rastercalculator", qgis_env = monqgis)

params <- params.default.rastercalculator

path <- "C:/Users/Bastien/Desktop/Codes corrections A/data/temp/"
params$INPUT_A <- paste0(path,"Ratio_BLDG.sdat")
params$INPUT_B <- paste0(path,"beta1_KR_IF_bldg.sdat")
params$OUTPUT <- paste0(path, "KR_bldg_transfo.tif")
params$FORMULA <- paste0(convertir_KR_vers_IF$beta_0_bldg[1],"+(A*B)")  
params$RTYPE <- "5"
system.time(run_qgis(alg = "gdalogr:rastercalculator", params=params, qgis_env = monqgis))

file.remove(list.files("C:\\Users\\Bastien\\Desktop\\Codes corrections A\\data\\temp\\", full.names = T))

Notice that it's what I think you call multipleinput, and it still works, despite you said you still had to deal with that.

Thank you again for you time

@jannes-m
Copy link
Collaborator

jannes-m commented Oct 6, 2017

I was referring to the QGIS input type multipleinput. See here and/or the example section of save_spatial_objects for an example

@pat-s
Copy link
Member

pat-s commented Oct 19, 2017

FYI: I tried your reprex

data("dem", package = "RQGIS")
raster::writeRaster(dem, file.path(tempdir(), "dem.tif"))
run_qgis(alg = "gdalogr:rastercalculator", 
                     INPUT_A = file.path(tempdir(), "dem.tif"),
                     OUTPUT = "depth_D500b.tif",
                     FORMULA = "A*2",
                     RTYPE = 1)
file.remove(list.files(tempdir(), full.names = TRUE))

with both master and unlock branch and I was able to remove the file in both cases on Mac.

@jannes-m
Copy link
Collaborator

Thanks for testing @pat-s. Thought so, that this is a Windows-only issue.

@pat-s
Copy link
Member

pat-s commented Nov 17, 2017

@jannes-m whats the status here?

@jannes-m
Copy link
Collaborator

jannes-m commented Dec 8, 2017

Well, I am not sure about possible side-effects. Therefore, I would leave the "unlocking procedure" in this branch as long as it is not thoroughly tested. And it seems @BastienFR is the only one so far needing this feature, therefore no rush to integrate it into master. Besides, maybe this issue will be resolved with the release of QGIS 3....

@lbusett
Copy link

lbusett commented Jan 24, 2018

Hi,

possibly related to this: today I was noticing a strange behaviour while trying to run algorithm "alg = "qgis:eliminatesliverpolygons".

In practice, when running multiple times the algorithm within the same R session but with different parameters, the input params were apparently not "updated" after the first run, and the result was always that of the first run. The only way to have "new" results seemed to be to restart the "R" session.

However, after installing the "unlock" branch the problem apparently disappeared: don't know if this is related to a similar issue, or to other differences with respect to the CRAN version.

I am working on Ubuntu 17.04, R 3.4.3

PS: really nice package! Thanks!

Ps2 : also possibly related to #74

@jannes-m
Copy link
Collaborator

Hi,
thanks for your input! This is really interesting. I will test if the unlock-branch indeed solves the problems under Linux. Under Windows everything works as expected, i.e. the input params become updated. In any case, this is a really strange behavior. But if we have a solution here, we can report this back to the QGIS team, asking them what is going amiss.

Pls note that I have updated now also the unlock branch, i.e., you will have all the new features we have added to the master. Again, thanks for reporting back, really helpful!

@jannes-m
Copy link
Collaborator

Well, indeed, the unlock branch resolves also #74. So this is a strong incentive to work further on the unlock-solution. Basically, this would solve:

@lbusett
Copy link

lbusett commented Jan 25, 2018

Glad it helped! Then for the moment I'll keep working on the "unlock" branch.

@lbusett
Copy link

lbusett commented Jan 26, 2018

Hi.

I dug in this a bit more since I had a hunch that there was a simple "file overwrite" problem. In fact, passing files as inputs in the example of #74 works properly:

spdf1_file <- tempfile(fileext = ".shp")
spdf2_file <- tempfile(fileext = ".shp")
spdf3_file <- tempfile(fileext = ".shp")

st_write(st_as_sf(SPDF1), spdf1_file)
st_write(st_as_sf(SPDF2), spdf2_file)
st_write(st_as_sf(SPDF3), spdf3_file)

test1 <- RQGIS::run_qgis("qgis:intersection", INPUT = spdf1_file, INPUT2 = spdf2_file, 
                         OUTPUT = file.path(tempdir(), "test1.shp"), 
                         load_output = TRUE)
test2 <- RQGIS::run_qgis("qgis:intersection", INPUT = spdf1_file, INPUT2 = spdf3_file, 
                         OUTPUT = file.path(tempdir(), "test2.shp"),
                         load_output = TRUE)

identical(test1, test2)
[1] FALSE

The problem appears to be in save_spatial_objects. Just changing line 463 from

fname <- file.path(tempdir(), paste0(names(params)[i], ".shp"))
to:

fname <- tempfile(fileext = ".shp")

solves the issue.

The culprit appears to be the call to write_sf in:

cap <- capture.output({
        suppressWarnings(
          test <-
            try(write_sf(params[[i]], fname, quiet = TRUE), silent = TRUE)
        )
      })

not throwing an error when trying to overwrite a file, because by default it uses "delete_layer = TRUE", and therefore not triggering the "catching error" mechanism immediately below. Therefore, also this simple change appears to solve the issue:

cap <- capture.output({
        suppressWarnings(
          test <-
            try(write_sf(params[[i]], fname, quiet = TRUE, delete_layer = FALSE), silent = TRUE)
        )
      })

However, I think that using the "tempfile" solution could be better, because it would prevent any "name clashing" problems. Only problem could be that tempfiles could clutter the tempdir(), but a cleanup solution could be easy to implement.

HTH

@jannes-m
Copy link
Collaborator

@lbusett Thanks for the input. I would have to take a look at this. In any case, it appears that write_sf() works differently under Windows and Linux. Under Windows it throws an error when trying to delete an already existing file (which here not works because QGIS impedes this). So thanks for clearing this up!

But what you propose in the end (deleting input files), probably only works with the unlock-branch, otherwise QGIS will prevent it.

In any case, I will think about using tempfile(fileext = ".shp") as you propose. This could make things easier (though less readable but that wouldn't be really important here since all this stuff is done internally...). Thanks again!

@lbusett
Copy link

lbusett commented Jan 26, 2018

You're welcome. Note that using st_write instead of write_sf appears to work the same way on windows and linux (i.e., always throwing an error on overwrite attempts, unless specified by the user).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants