[go: nahoru, domu]

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

-Issue: #205: Merging (Identically Specified) MinHashLSH objects #232

Merged
merged 6 commits into from
Mar 12, 2024

Conversation

rupeshkumaar
Copy link
Contributor

Merging two MinHashLSH of the same length (num_perm).

@rupeshkumaar
Copy link
Contributor Author

@ekzhu I have raised a PR. I have tried to cover all the points that we had discussed. Please feel free to engage and let me know if any modification is required. Thank you!

datasketch/lsh.py Outdated Show resolved Hide resolved
@rupeshkumaar
Copy link
Contributor Author

@ekzhu Resolved the conversations. Can you please go through the changes. Thank you. Let me know if there are any other changes.

@ekzhu
Copy link
Owner
ekzhu commented Jan 22, 2024

Could you also add "3.12" to the tested python version:

https://github.com/ekzhu/datasketch/blob/master/.github/workflows/test.yml#L11

datasketch/lsh.py Outdated Show resolved Hide resolved
datasketch/lsh.py Outdated Show resolved Hide resolved
@junrae6454
Copy link

@ekzhu When will this commit be merged?

@ekzhu
Copy link
Owner
ekzhu commented Mar 1, 2024

@junrae6454 Still waiting for @rupeshkumaar's changes. Unless you want to help finishing this?

@rupeshkumaar
Copy link
Contributor Author

Could you also add "3.12" to the tested python version:

https://github.com/ekzhu/datasketch/blob/master/.github/workflows/test.yml#L11

This is also done.

@ekzhu
Copy link
Owner
ekzhu commented Mar 2, 2024

Could you also add "3.12" to the tested python version:
https://github.com/ekzhu/datasketch/blob/master/.github/workflows/test.yml#L11

This is also done.

Thanks. Let me know when the changes are pushed

@rupeshkumaar
Copy link
Contributor Author

Please do let me know, @ekzhu about the hashfunc for redis that I have asked. Thank you. Logging out for now. Once this is done I will push the changes.

@rupeshkumaar
Copy link
Contributor Author

@ekzhu Please review.

datasketch/lsh.py Outdated Show resolved Hide resolved
datasketch/lsh.py Show resolved Hide resolved
datasketch/lsh.py Outdated Show resolved Hide resolved
docs/lsh.rst Outdated Show resolved Hide resolved
@ekzhu
Copy link
Owner
ekzhu commented Mar 11, 2024

Also, to fix the tests failure for now, remove 3.12 from python version.

@rupeshkumaar
Copy link
Contributor Author

Please review @ekzhu

@ekzhu ekzhu merged commit a532f06 into ekzhu:master Mar 12, 2024
6 checks passed
@ekzhu
Copy link
Owner
ekzhu commented Mar 12, 2024

@rupeshkumaar merged, thank you for the hard work!!

@ekzhu
Copy link
Owner
ekzhu commented Mar 12, 2024

@rupeshkumaar if you are interested, you can take a look at this issue #236 that broke the library for python 3.12

@rupeshkumaar
Copy link
Contributor Author

Yeah, sure I will take a look. @ekzhu

@rupeshkumaar rupeshkumaar deleted the rupeshkumaar/issue205 branch March 12, 2024 17:14
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.

None yet

3 participants