search

image-rs / imageproc

Image processing operations
MIT License
758 stars 149 forks source link

Refactor `gradients` #660

Closed cospectrum closed 5 months ago

cospectrum commented 5 months ago

@theotherphil Did you approve deprectaed macro? I don't like it. And gradients_grayscale hard to use because there are 3 unused generics

ripytide commented 5 months ago

I agree with removing the unused generics, I don't know how they got there. I disagree with removing the deprecation attributes as all those functions are inflexible and pollute the docs, especially if they ever got _parallel() variants.

cospectrum commented 5 months ago

functions are inflexible and pollute the docs

And now we will pollute the user's codebase with long upper case variables and reimplementations

ripytide commented 5 months ago

I think it's more readable as well tbh. Easier than having to look up the docs of one of 20 weirdly named functions every time I encounter one. I only have to learn one function filter_clamped and then I as a reader will understand every call to it.

theotherphil commented 5 months ago

I’ve created https://github.com/image-rs/imageproc/issues/661 to discuss what API we actually want for filtering. So we can agree this in one place rather than going back and forth in pull requests. (I’m also tempted to name filter_clamped just filter as it seems the more useful function so deserves a shorter name.) I don’t want to keep churning function names on every release as this will cause repeated breaking changes for users and there are for more users than maintainers!

@cospectrum @ripytide I’ll merge this PR but will get consensus on the linked issue before releasing the next crate version.