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

Fix coercion to lower case when clean = TRUE #105

Merged
merged 3 commits into from
Feb 12, 2024
Merged

Fix coercion to lower case when clean = TRUE #105

merged 3 commits into from
Feb 12, 2024

Conversation

etiennebacher
Copy link
Collaborator

@etiennebacher etiennebacher commented Feb 12, 2024

The docs say

should the strings that you fuzzy join on be cleaned (coerced to lower-case, stripped of punctuation and spaces)? Default is FALSE

but strings were actually not coerced to lowercase (unless I missed something happening on the Rust side). This PR fixes that. Unfortunately I don't really see a good way to test this.

@beniaminogreen
Copy link
Owner

Thanks for catching this, I will merge in very shortly.

Would you like to be added as a collaborator for this repo? You strike me as a very experienced R / Rust dev than I, and I the project would benefit from you being able to write, review, and merge changes directly.

@beniaminogreen beniaminogreen merged commit e602cb9 into beniaminogreen:main Feb 12, 2024
7 checks passed
@etiennebacher etiennebacher deleted the coerce-to-lowercase branch February 12, 2024 16:27
@etiennebacher
Copy link
Collaborator Author

Would you like to be added as a collaborator for this repo? You strike me as a very experienced R / Rust dev than I, and I the project would benefit from you being able to write, review, and merge changes directly.

That would be great! Note that if you want to retain some control over what gets merged, you can modify the repo settings to require approval before merging.

I have some experience as R dev but I'm only beginner in Rust, there's no way I could have built this package 😄. I also have much less experience regarding those methods (I basically discovered LSH with your package)

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

2 participants