search

ocean-tracking-network / glatos

9 stars 4 forks source link

Missing "srs" slot in greatLakesTrLayer #213

Closed mhpob closed 9 months ago

mhpob commented 10 months ago

On the current dev branch the vdat_list Extract method prevents devtools::document from working and produces the following error:

==> devtools::document(roclets = c('rd', 'collate', 'namespace'))

ℹ Updating glatos documentation
ℹ Loading glatos
Error in match(x, table, nomatch = 0L) : 
  no slot of name "srs" for this object of class "TransitionLayer"
Calls: suppressPackageStartupMessages ... roclet_process.roclet_namespace -> warn_missing_s3_exports -> %in%
Execution halted

Exited with status 1.

If I remove the code specifying the method, everything works well. If I put any code in specifying any method, it breaks.

I'm a bit confused as this has obviously not prevented someone else from running it: the S3method is exported in NAMESPACE (this commit); and none of the methods have anything to do with any TransitionLayers. The only thing I could find to reproduce the error is with greatLakesTrLayer:

> greatLakesTrLayer@srs
Error: no slot of name "srs" for this object of class "TransitionLayer"

However, the original error suggests this is an issue with match/%in% and I can't find anything using those on a TransitionLayer in the current code.

At a loss on this one and thinking it may just be on my computer. I apologize in advance if this is a wild goose chase.

chrisholbrook commented 10 months ago

I can't reproduce the issue locally on current dev branch.

mhpob commented 10 months ago

Thanks for checking. No idea what's going on here.

chrisholbrook commented 9 months ago

I just pulled dev (from a5f0b1b94af3fe488d9abb366e7a3d7109fb5da2) and now get this error when I run devtools::check():

devtools::check() ══ Documenting ═══════════════════════════════════════════════════════════════════════════════════════════════════════════ ℹ Updating glatos documentation ℹ Loading glatos version 0.8.0.9003 ('very-refreshing-lemonade') Error in match(x, table, nomatch = 0L) : no slot of name "srs" for this object of class "TransitionLayer"

@mhpob Any resolution on your end?

mhpob commented 9 months ago

No, I figured it was system specific since you weren't able to re-create it before. It looks like the document GH Action fails with that, too.

For whatever reason, the GH Action R CMD check works (see recent runs)... And that's directly running the CLI check that devtools wraps.

On Thu, Feb 15, 2024, 8:46 PM chrisholbrook @.***> wrote:

I just pulled dev (from a5f0b1b https://github.com/ocean-tracking-network/glatos/commit/a5f0b1b94af3fe488d9abb366e7a3d7109fb5da2) and now get this error when I run devtools::check():

devtools::check() ══ Documenting ═══════════════════════════════════════════════════════════════════════════════════════════════════════════ ℹ Updating glatos documentation ℹ Loading glatos version 0.8.0.9003 ('very-refreshing-lemonade') Error in match(x, table, nomatch = 0L) : no slot of name "srs" for this object of class "TransitionLayer"

@mhpob https://github.com/mhpob Any resolution on your end?

— Reply to this email directly, view it on GitHub https://github.com/ocean-tracking-network/glatos/issues/213#issuecomment-1947632576, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6Y23JZUUGAMEOKUHAMV5TYT227JAVCNFSM6AAAAABCJSVLUKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBXGYZTENJXGY . You are receiving this because you were mentioned.Message ID: @.***>

mhpob commented 9 months ago

I think the document GH Action runs devtools rather than the CLI program, so I'm curious if it's an issue with devtools

mhpob commented 9 months ago

Looks like it's actually an issue with roxygen2:: roxygenize(roclet = 'namespace'), which is what devtools wraps

chrisholbrook commented 9 months ago

Thanks for the tip. That lead me to discover that the issue seems unrelated to the extract method for vdat_list, but rather something about greatLakesTrLayer causing the issue.

Factoring out the extract method, I can produce the error message with:

1 %in% list(greatLakesTrLayer)

Error in match(x, table, nomatch = 0L) : no slot of name "srs" for this object of class "TransitionLayer"

Any value in place of 1 will produce same error.

This seems related to the extract method only because the extract method is the only S3 method exported from this package, so when it's absent methods %in% s3objects doesn't get called.

Seems similar issues with RasterLayer objects have been reported (e.g., https://stackoverflow.com/questions/76074839/error-error-c-stack-usage-is-too-close-to-the-limit) but no obvious cause or solution.

@haydento I think this points to revisiting that greatLakesTrLayer object, maybe rebuilding it.

chrisholbrook commented 9 months ago

Which of these things is not like the others?

image

@haydento Could you revisit this greatLakesTrLayer example data file, check for compatibility with contemporary R spatial packages?

Looking at str(greatLakesTrLayer), I wonder if this is an sp legacy issue (also note that the warning at the bottom is the same message that triggered this issue):

image

Ideally we would also have a script in data-raw showing how this example file was created.

mhpob commented 9 months ago

Looking at some other issues... ~Was that maybe a terra SpatRaster that was forced through a function expecting a raster RasterLayer?~ (nevermind, just saw it was created in 2017 which predates most of terra)

Or created with outdated packages?

https://stackoverflow.com/questions/75489041/error-for-create-slope-cs-function-in-the-leastcostpath-package

mhpob commented 9 months ago

@chrisholbrook with regard to it working for you before and not now, did you happen to update your version of roxygen recently? Or perhaps the updated DESCRIPTION forced you to?

Looking at the commit history of the Roxygen Note line of DESCRIPTION, the last time you touched it you were using version 7.2.3 and had no issues. That may have been the version you were using before when you weren't able to recreate the problem. When I documented at some point it bumped to v7.3.1.

If cleaning the transition layer doesn't bear fruit, there may be something to be found in the way Roxygen changed handling S3/Namespace between those two versions: https://roxygen2.r-lib.org/news/index.html

chrisholbrook commented 9 months ago

Yes, I updated roxygen2 7.3.1 from 7.2.3 yesterday before I ran devtools::check, so that explanation makes sense why I didn't see that earlier.

mhpob commented 9 months ago

As a quick fix, it looks like you can use the Proj.4 string in greatLakesTrLayer@crs@projargs to fill in the @srs slot without rebuilding/recreating the data from whole cloth. The srs slot seems to hold the WKT form of a given CRS. The TransitionLayer class inherits from raster::RasterLayer, which requires an srs slot. When running checks, roxygen v7.3.1 apparently can now see this as an invalid object since the greatLakesTrLayer@srs slot does not exist.

The code below compares greatLakesTrLayer and an srs-backfilled gLTL against transition layers made in the examples. Note that the WKT is not exactly the same as the examples since greatLakesTrLayer is specified with "+proj=longlat +ellps=WGS84 +towgs84=0,0,0,0,0,0,0 +no_defs" and the others via EPSG:4326, which is "+proj=longlat +datum=WGS84 +no_defs ".

> library(glatos)
version 0.8.0.9003 ('very-refreshing-lemonade')
> library(testthat)
> 
> ## greatLakesTrLayer fails with no SRS slot
> expect_error(
+   grepl("GEOGCRS", greatLakesTrLayer@srs), 
+   "no slot of name \"srs\" for this object of class \"TransitionLayer\""
+ )
> 
> ## Others pass with SRS containing WKT string
> tst1_sf <- make_transition(great_lakes_polygon, res = c(0.1, 0.1))
Warning message:
This function is deprecated and will be removed in the next version 
> tst1_sp <- make_transition(greatLakesPoly, res = c(0.1, 0.1))
Warning messages:
1: This function is deprecated and will be removed in the next version 
2: CRS of input was not EPSG:4326, so conversion was attempted. 
> 
> expect_true(
+   grepl("GEOGCRS", tst1_sf$transition@srs)
+ )
> expect_true(
+   grepl("GEOGCRS", tst1_sp$transition@srs)
+ )
> 
> tst2_sf <- make_transition2(great_lakes_polygon, res = c(0.1, 0.1))
Making transition layer...
Done.
Warning message:
This function is deprecated and will be removed in the next version 
> tst2_sp <- make_transition2(greatLakesPoly, res = c(0.1, 0.1))
Making transition layer...
Done.
Warning message:
This function is deprecated and will be removed in the next version 
> 
> expect_true(
+   grepl("GEOGCRS", tst2_sf$transition@srs)
+ )
> expect_true(
+   grepl("GEOGCRS", tst2_sp$transition@srs)
+ )
> 
> tst3_sf <- make_transition3(great_lakes_polygon, res = c(0.1, 0.1))
Making transition layer...
Done.
> tst3_sp <- make_transition3(great_lakes_polygon, res = c(0.1, 0.1))
Making transition layer...
Done.
> 
> expect_true(
+   grepl("GEOGCRS", tst3_sf$transition@srs)
+ )
> expect_true(
+   grepl("GEOGCRS", tst3_sp$transition@srs)
+ )
> 
> ## Use greatLakesTrLayer CRS to make SRS (WKT)
> gLTL <- greatLakesTrLayer
> 
> gLTL@srs <- raster::crs(greatLakesTrLayer@crs@projargs)
> 
> cat(gLTL@srs)
BOUNDCRS[
    SOURCECRS[
        GEOGCRS["unknown",
            DATUM["World Geodetic System 1984",
                ELLIPSOID["WGS 84",6378137,298.257223563,
                    LENGTHUNIT["metre",1]],
                ID["EPSG",6326]],
            PRIMEM["Greenwich",0,
                ANGLEUNIT["degree",0.0174532925199433],
                ID["EPSG",8901]],
            CS[ellipsoidal,2],
                AXIS["longitude",east,
                    ORDER[1],
                    ANGLEUNIT["degree",0.0174532925199433,
                        ID["EPSG",9122]]],
                AXIS["latitude",north,
                    ORDER[2],
                    ANGLEUNIT["degree",0.0174532925199433,
                        ID["EPSG",9122]]]]],
    TARGETCRS[
        GEOGCRS["WGS 84",
            DATUM["World Geodetic System 1984",
                ELLIPSOID["WGS 84",6378137,298.257223563,
                    LENGTHUNIT["metre",1]]],
            PRIMEM["Greenwich",0,
                ANGLEUNIT["degree",0.0174532925199433]],
            CS[ellipsoidal,2],
                AXIS["latitude",north,
                    ORDER[1],
                    ANGLEUNIT["degree",0.0174532925199433]],
                AXIS["longitude",east,
                    ORDER[2],
                    ANGLEUNIT["degree",0.0174532925199433]],
            ID["EPSG",4326]]],
    ABRIDGEDTRANSFORMATION["Transformation from unknown to WGS84",
        METHOD["Geocentric translations (geog2D domain)",
            ID["EPSG",9603]],
        PARAMETER["X-axis translation",0,
            ID["EPSG",8605]],
        PARAMETER["Y-axis translation",0,
            ID["EPSG",8606]],
        PARAMETER["Z-axis translation",0,
            ID["EPSG",8607]]]]
chrisholbrook commented 9 months ago

Good call. Fixed in 0.8.0 (https://github.com/ocean-tracking-network/glatos/commit/f016ab1538634e6368da7b80411046d2251ef253) with greatLakesTrLayer@srs <- raster::crs(greatLakesTrLayer@crs@projargs).

See also #219