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

First draft for in_linter() #2621

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

First draft for in_linter() #2621

wants to merge 1 commit into from

Conversation

Bisaloo
Copy link
Contributor

@Bisaloo Bisaloo commented Jun 22, 2024

First attempt to fix #1875.

Questions:

  • Should scalar_in_linter() be merged with this linter? I see these two linters are counterpart to one another.

Unaddressed cases:

  • Mixed chains such as x == 'a' | y == 'b' | x == 'c' are not covered at this time but I would assume most people at least group the tests by variable(?) so it's maybe okay to leave it for now.

@AshesITR
Copy link
Collaborator

AshesITR commented Jun 24, 2024

I think it might be nice to be able to tweak a minimum nesting depth, i.e., number of repeated | x == "val" expressions, especially since experimentally, the two-component case is slower with %in%.

Also note there is different behavior with respect to NA inputs.

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.

New linter to recommend using %in%
2 participants