poissonconsulting / fwatlasbc

A R package to get data from the Freshwater Atlas of British Columbia
https://poissonconsulting.github.io/fwatlasbc/
Other
9 stars 0 forks source link

Checking fwatlasbc #70

Closed dunkenwg closed 1 week ago

dunkenwg commented 1 month ago

@joethorley, the actions are failing because four tests are failing:

Failure (test-add-collection-to-polygon.R:54:3): fwa_add_collection_to_polygon function intersects work
x$geometry[[1]] inherits from 'XY'/'POINT'/'sfg' not 'LINESTRING'.

Failure (test-add-collection-to-polygon.R:55:3): fwa_add_collection_to_polygon function intersects work
colnames(sf::st_coordinates(x$geometry[[1]])) (`actual`) not identical to c("X", "Y", "L1") (`expected`).

`actual`:   "X" "Y"     
`expected`: "X" "Y" "L1"

Failure (test-snap-rm-to-rms.R:412:3): fwa_snap_rm_to_rms interpolates blocks
x$new_rm (`actual`) not equal to c(1, 2, 3, 4, 5, 6, 7, 8, 10, 11, 12, 13, 13, 14, 15) (`expected`).

  `actual`: 1 2 2 3 4 6 6 7  9 10 and 5 more...
`expected`: 1 2 3 4 5 6 7 8 10 11           ...

Failure (test-snap-rm-to-rms.R:413:3): fwa_snap_rm_to_rms interpolates blocks
x$distance_to_new_rm (`actual`) not equal to c(...) (`expected`).

  `actual`: 0 0 0 874 0 0 932 0   0 0 and 5 more...
`expected`: 0 0 0   0 0 0   0 0 536 0           ...

We're also getting 46 instances of the same warning:

fwa_snap_rms_to_rms multiple blks to 1 blk partial argument match of 'rm' to 'rms'

Would we be able to discuss the functions and tests of this package so that I have a better idea of how to fix the tests?

dunkenwg commented 1 month ago

@joethorley, would doing styler changes after having the rest of the changes reviewed work?

Also, some of the snapshots are changing (in add-gm-elevation-to-point and convert-streams-to-rms) and causing tests to fail.

Some possibly good news is that neither clicking the test button (instead of running devtools::test()) or running the actions causes any fwa_snap_rms_to_rms multiple blks to 1 blk partial argument match of 'rm' to 'rms' warnings.

joethorley commented 3 weeks ago

@dunkenwg - yes I think you should first do a PR that only fixes the breaking tests and once are happy with that we can make the other changes.