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

Consolidate suboptimal usages of which() under one linter #2684

Closed
wants to merge 9 commits into from

Conversation

IndrajeetPatil
Copy link
Collaborator

@IndrajeetPatil IndrajeetPatil commented Nov 5, 2024

Closes #1544

@IndrajeetPatil IndrajeetPatil requested review from AshesITR and MichaelChirico and removed request for AshesITR November 5, 2024 22:19
@AshesITR
Copy link
Collaborator

Isn't which.min(x) only the first index of a minimum value?

x <- c(1, 1)
which(x == min(x))
which.min(x)

@IndrajeetPatil
Copy link
Collaborator Author

IndrajeetPatil commented Nov 21, 2024

Oh damn, you are right!
I didn't consider that using which(x == min(x)) to find all minimum elements, and not just the (first) one, is also a valid and intentional usage.

I am not sure how to proceed then. Just looking at this code then we can't guess the intent, and it's not possible to reliably suggest an alternative.

@AshesITR
Copy link
Collaborator

I would forfeit this linter unless we find a more efficient and still equivalent function. It is algorithmically possible with a single pass of x, but I'm not aware of any implementations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consolidate suboptimal usages of which() under one linter
2 participants