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

boolean-positional-value-in-call (FBT003) - don't warn when function name starts with set_ and only takes one argument #8923

Closed
DetachHead opened this issue Nov 30, 2023 · 7 comments · Fixed by #9429 or #11287
Assignees
Labels
rule Implementing or modifying a lint rule

Comments

@DetachHead
Copy link

functions like these should not report the error imo, since the meaning of the bool is self explanatory, which is often the case when the functin starts with set_:

def set_enabled(enabled: bool):
    ...

set_enabled(True) # self explanatory
set_enabled(enabled=True) # unnecessary/redundant
@zanieb
Copy link
Member

zanieb commented Nov 30, 2023

This seems a little bit too specific to me.

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Jan 8, 2024
@charliermarsh
Copy link
Member

We may want to do this, it would also help with #9287 and #3247 (comment).

@charliermarsh charliermarsh self-assigned this Jan 8, 2024
charliermarsh added a commit that referenced this issue Jan 8, 2024
## Summary

Ignores Boolean trap enforcement for methods that appear to be setters
(as in the Qt and pygame APIs).

Closes #9287.
Closes #8923.
@tmke8
Copy link
Contributor

tmke8 commented Jan 18, 2024

Maybe also allow the use_ prefix? I encountered that with torch.use_deterministic_algorithms. The following feels a bit silly:

torch.use_deterministic_algorithms(mode=True)

@charliermarsh
Copy link
Member

I'm kind of wondering if we should just avoid calling this on third-party function calls.

@bswck
Copy link
Contributor

bswck commented Apr 16, 2024

I often use flag-holding context vars.

from contextvars import ContextVar

cv = ContextVar("cv", default=False)
cv.set(True)  # triggers FBT003

What a pity the fix for this doesn't apply to ContextVar.set.

@DetachHead
Copy link
Author

it looks like the check expects the function name to start with "set" followed by either an underscore or an uppercase character, but not "set" on its own. it should probably be updated to allow that too.

@zanieb
Copy link
Member

zanieb commented Apr 16, 2024

It seems reasonable to expand it to cover that case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
5 participants