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

Import collapse to reduce the time taken by %in% #104

Merged
merged 4 commits into from
Feb 12, 2024
Merged

Import collapse to reduce the time taken by %in% #104

merged 4 commits into from
Feb 12, 2024

Conversation

etiennebacher
Copy link
Collaborator

Adds collapse::%!iin% calls instead of !%in%.

In addition to that, missing indices in a and b shouldn't be always computed (similarly to #101). For instance, if mode == "left" then not_matched_b is not used. I took the liberty of converting the multiple if else to a switch() call because I find that clearer than nested if else, but no problem to change if you prefer keeping the previous way.

Close #102

@etiennebacher
Copy link
Collaborator Author

Regarding the version in DESCRIPTION, it was 0.1.4 but the CRAN version is 0.1.2. @beniaminogreen did you use 0.1.4 as a development version? I changed that to 0.1.2.9000 which is a more common pattern to signal development versions, but again no problem to change if necessary.

@beniaminogreen
Copy link
Owner

Thanks for this, and nice catch with the development version. I did not know that that was the convention to show development versions. I'll let the tests go through and wait a few mins in case there are any other tweaks you would like to make, then I will merge it in!

@etiennebacher
Copy link
Collaborator Author

I did not know that that was the convention to show development versions

Well at least it's the tidyverse convention and many packages use it: https://r-pkgs.org/lifecycle.html#sec-lifecycle-version-number-tidyverse

Some people also bump the 9000 after every change in the development version, so that it's easier to differentiate 0.1.2.9015 and 0.1.2.9008 for example.

In any case, I'm done with this PR so feel free to merge when you want

@beniaminogreen beniaminogreen merged commit c15ae70 into beniaminogreen:main Feb 12, 2024
7 checks passed
@etiennebacher etiennebacher deleted the more-speedup branch February 12, 2024 14:35
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.

Suggestion: use collapse::%!iin% instead of !%in%
2 participants