Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor gradients #660

Merged
merged 3 commits into from
May 28, 2024
Merged

Refactor gradients #660

merged 3 commits into from
May 28, 2024

Conversation

cospectrum
Copy link
Contributor

@cospectrum cospectrum commented May 27, 2024

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

@ripytide
Copy link
Member

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
Copy link
Contributor Author

cospectrum commented May 27, 2024

functions are inflexible and pollute the docs

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

@ripytide
Copy link
Member

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
Copy link
Contributor

I’ve created #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.

@theotherphil theotherphil merged commit f5d9ef3 into image-rs:master May 28, 2024
15 checks passed
@cospectrum cospectrum deleted the gradients branch September 13, 2024 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants