Implement more Testing using {tinytest}.
Examples of this include the following:
- Add tests that verify the default arguments of PH_pointcloud() and/or PH_raster()
- Add testing for the property validator itself ("only these library names are allowed")
- Add testing for accepted values - We currently have very few tests that say "This should succeed", but have so many that say "This should fail". For example, library_type explicitly allows "GUDHI", "PHAT", "Dionysus", NA_character_. The validator promises these are valid, but its never verified that they actually construct successfully.
- Add testing for sublevel values - we already have
expect_error( PH_raster(sublevel = NA), "TRUE or FALSE" ) but we dont have testing for sublevel = "TRUE" or sublevel = 1, in which both instances are wrong types
- Add testing for max_diameter, max_radius, and max_scale functions - Right now max_diameter, max_radius, and max_scale do not have range validation implemented. So currently, the package does not claim that negative values are invalid (which is mathematically questionable ?)
- etc.
As of right now, I would not test negative values because it would be asserting a behavior that the package does not currently implement. So, if we were to potentially add expect_error( PH_pointcloud(max_diameter = -1), "non-negative" ), the test would fail and then you would either modify the package code, or remove the test. For this particular GitHub issue, the safer contribution is expanding coverage of existing behavior, but this is definitely something to consider in the future.
Implement more Testing using {tinytest}.
Examples of this include the following:
expect_error( PH_raster(sublevel = NA), "TRUE or FALSE" )but we dont have testing forsublevel = "TRUE"orsublevel = 1, in which both instances are wrong typesAs of right now, I would not test negative values because it would be asserting a behavior that the package does not currently implement. So, if we were to potentially add
expect_error( PH_pointcloud(max_diameter = -1), "non-negative" ), the test would fail and then you would either modify the package code, or remove the test. For this particular GitHub issue, the safer contribution is expanding coverage of existing behavior, but this is definitely something to consider in the future.