enable full info() on vector-via-sql #615
Conversation
R/gdalvector.R
Outdated
| #' prints the DSN and SQL. | ||
| #' `cl_arg = c("-so", "-nomd")`, and for layers open with a SQL statement, | ||
| #' calls [ogrinfo()] passing the command options | ||
| #' `cl_arg = c("-so", "-nomd", "-sql", <layer>)`. |
There was a problem hiding this comment.
Really minor I know, but instead of ... "-sql", <layer>)I'm thinking it should say ... "-sql", <statement>), since that would match the ogrinfo command-line documentation at https://gdal.org/en/stable/programs/ogrinfo.html#ogrinfo (and you pass R_NilValue for the layer and m_layer_name in this case actually contains the sql statement).
Acknowledge it's not exactly clean relative to the GDALVector constructor where we only have layer which can be layer name or SQL statement, but since these are ogrinfo cl options I sort of think it should match to that. Your call, feel free to keep it the way it is.
There was a problem hiding this comment.
great catch, I didn't think to look at ogrinfo
| @@ -1079,7 +1079,7 @@ test_that("info() prints output to the console", { | |||
| lyr$close() | |||
|
|
|||
| lyr <- new(GDALVector, dsn, "SELECT * FROM mtbs_perims LIMIT 10") | |||
| expect_output(lyr$info()) | |||
| expect_output(lyr$info(), "Feature Count: 10") | |||
There was a problem hiding this comment.
This makes me realize I have several expect_output that are missing some specific text to check for! Will fix at some point.
There was a problem hiding this comment.
I was very happy to see that expect_output was as sophisticated as it is ;)
I'll include the edit as I've been working with that already. |
Implementation of #614