test-custom_functions.R errors if sf adds attributes to sf objects

[mikemahoney218]

Hi there!

As currently written, test-custom_functions.R is partially testing that the underlying sf object never changes. Specifically, the expect_identical() here (second to last) will throw errors if sf ever changes the attributes attached to sf objects:

h3jsr/tests/testthat/test_custom_functions.R

Lines 35 to 54 in 9ee5b62

	test_that(
	'cell_to_line returns correctly - data.frame with list-column',
	c(library(sf),
	brisbane_hex_10 <- cell_to_polygon(input = '8abe8d12acaffff'),
	hex_sample <- get_disk_list('8abe8d12acaffff', 4)[[1]][[4]][seq(1,18,3)],
	hex_sample_polys <- cell_to_polygon(hex_sample),
	paths <- grid_path(rep('8abe8d12acaffff', 6), hex_sample),
	paths_df <- data.frame('ID' = seq(6), 'paths' = I(paths)),
	val1 <- cell_to_line(paths_df),
	val2 <- cell_to_line(paths_df, simple = FALSE),
	expect_is(val1, 'sfc_LINESTRING'),
	expect_length(val1, 6),
	expect_is(val2, 'sf'),
	expect_equal(ncol(val2), 3),
	expect_equal(nrow(val2), 6),
	ins <- sf::st_set_geometry(val2, NULL), # wierd behav when lib() missing
	expect_identical(ins, paths_df),
	expect_is(val2$geometry, 'sfc_LINESTRING')
	)
	)

The issue is that by using expect_identical(), testthat is partially testing if your objects contain exactly the same attributes. Because you're comparing against old saved sf objects, that means you're partially testing internal implementation details of the sf object that aren't guaranteed to remain the same over time. There's a bit more about how testing the attributes of objects created by other packages can be a problem on the Tidyverse blog.

At the moment, we're trying to add a new attribute to sf objects, which causes these tests to error. I'm writing to ask if you'd consider changing these tests to allow sf to improve the internal structure of sf objects.

I believe this test is looking to confirm that the ID and paths of these two objects are equal to each other. Would it be possible to change this test to expect_equal() with the check.attributes argument set to FALSE?

expect_equal(ins, paths_df, check.attributes = FALSE)

This change would mean this package is no longer blocking sf from making changes to the internal structure of sf objects, and would fix the immediate issue with the new attribute.

If you're open to it, I'd be happy to send you a PR to make these changes!

[obrl-soil]

Hi - yep happy to look over a PR for this - just can't get to it until this weekend.

[mikemahoney218]

Thanks!