search

image-rs / imageproc

Image processing operations
MIT License
736 stars 144 forks source link

Add `proptests` for `filter/mod.rs` #599

Closed cospectrum closed 3 months ago

ripytide commented 3 months ago

Looks good to me. As an aside, I wonder if there are any functions in the library that are perfect inverses on one another as that would make for some really neat property tests, testing for equality after applying a function and then its inverse.

cospectrum commented 3 months ago

I only see the reference implementation for vertical and horizontal filters (u8) above. Basically, I just want to understand if we have overflow, out of bounds, or any kind of panic. I have already documented some things.

cospectrum commented 3 months ago

There will also be more checks under sanitizers or with debug_assert. Miri is too slow for this amount of data

theotherphil commented 3 months ago

Thanks! It’s a bit clunky to have proptest and quickcheck used in the same crate - would be nice to move the quickcheck uses across at some point.

cospectrum commented 3 months ago

I think proptest is a more powerful lib, I can get rid of quickchek, but its in the public api behind the feature.

theotherphil commented 3 months ago

I expect it’s not widely used and I’m fine with the API change if it’s easy for users to migrate.

cospectrum commented 3 months ago

Why do we even have property_testing in the public api?

cospectrum commented 3 months ago

It's not used in integration tests, so it doesn't make sense to me. I would delete it altogether.

theotherphil commented 3 months ago

It's in the public API to help users test their own functions. It's been used in at least one other crate but I'm not sure how widely it's used now.