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

Add PDQHash and PDQHashF to Public Image Hashers #59

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mjs2600
Copy link
Contributor

@mjs2600 mjs2600 commented Oct 15, 2024

PDQHash and PDQHashF were unintentionally left off of the list of public hashers. This adds them to the list.

`PDQHash` and `PDQHashF` were unintentionally left off of the list of public hashers. This adds them to the list.
@mjs2600 mjs2600 requested a review from davidrs October 15, 2024 12:38
@davidrs
Copy link
Contributor

davidrs commented Oct 15, 2024

README may need an update since it says a different path pdq

@faustomorales
Copy link
Contributor

FWIW, leaving PDQ out was intentional. The reason was that pdqhash-python doesn't always build reliably and I didn't want someone to be blocked from using perception just because they couldn't get pdqhash-python to work. I believe that the change in this PR, as it stands, would make it impossible to use perception without pdqhash-python being installed successfully. But maybe that's what's best! I just wanted to make sure that outcome was considered.

@mjs2600
Copy link
Contributor Author

mjs2600 commented Oct 15, 2024

Thanks for the context @faustomorales! I'll rework this and move pdq into an extra in case someone doesn't want or need it.

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