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

Let's drop --strict-boolean #3195

Closed
gvanrossum opened this Issue Apr 19, 2017 · 8 comments

Comments

Projects
None yet
5 participants
@gvanrossum
Member

gvanrossum commented Apr 19, 2017

See discussion in #3185 -- it's not working perfectly and it is encouraging bad style. It also makes --strict less useful: it means you can't use --strict if you don't believe in --strict-optional, else you'd get a ton of false positive from the latter.

@gvanrossum

This comment has been minimized.

Member

gvanrossum commented Apr 19, 2017

Or at least start deprecating it and remove it from --strict.

@pkch

This comment has been minimized.

Contributor

pkch commented Apr 19, 2017

Before we proceed with this, I just wanted to clear up one thing based on the earlier discussion on gitter.

Would this flag become useful if it was restricted just to the cases where it's actually unsafe? Specifically the usage of non-boolean variable in a boolean context is unsafe only if it has more than 1 values that is interpreted as False:

def f(x: int) -> None:
  if x:  ... # safe
 
def g(x: int = None) -> None:
  if x:  ... # unsafe; is x 0 or None?
 
def h(x: set) -> None:
  if x: ... # safe

def i(x: set = None) -> None:
  if x: ... # unsafe; is x an empty set or None?

def j(x: Callable[[int], int] = None) -> None:
  if x: ... # safe

def k(x: bool = None) -> None:
  if x: ... # unsafe; is x False or None?

It seems very easy to identify unsafe cases, using the truth value testing rules. The usage is unsafe if the variable type is a Union that includes more than one of the following types:

  • None
  • a numeric type
  • a subtype of Sized
  • a type that has __bool__ method defined

(Edit: The last bullet point can wait until we have structural types)

Do you want me to check if restricting --strict-boolean to just these cases would make sense, perhaps using mypy own source code?

@JukkaL

This comment has been minimized.

Collaborator

JukkaL commented Apr 19, 2017

Agreed -- it's not worth having --strict enable it, and the entire option is questionable.

@pkch We can discuss that as a separate feature (which might perhaps share a name with the current option). Feel free to open an issue for it.

@JelleZijlstra

This comment has been minimized.

Collaborator

JelleZijlstra commented Apr 19, 2017

When --strict-boolean was added I tried it on my codebase and found it way too pedantic to be useful. Maybe some people really like writing if len(lst) > 0: instead of if lst:, but I don't think mypy needs to enforce that.

However, there are some related checks that mypy could do that are actually useful, like warning if you do if some_object.is_foo instead of if some_object.is_foo() (where is_foo is a method). But those kinds of things can be discussed in a separate issue.

@gvanrossum

This comment has been minimized.

Member

gvanrossum commented Apr 19, 2017

Regarding the idea that conflating an empty something (or zero) with None is a bad idea, it is actually quite common to have code that does this intentionally, and uglifying it just doesn't seem right.

So let someone please volunteer for a PR that deprecates it and removes it from the --strict set. (Explicit --strict-boolean or the corresponding config flag should still work for one more release, but with a deprecation message.)

@viktor25

This comment has been minimized.

viktor25 commented May 18, 2017

I use --strict-boolean, but I did not see this thread until I read the 0.510 release notes. Is the decision to remove it set in stone? Is there any possibility to leave it, possibly under a less pretentious name (e.g. --insane-boolean), and not as part of --strict?

Alternatively, is there an easy way to create a mypy wrapper that adds back the functionality?

@JukkaL

This comment has been minimized.

Collaborator

JukkaL commented May 18, 2017

@viktor25 The feature as it was implemented was problematic, since it actually doesn't do the right thing in all cases (as discussed in #3185 and elsewhere). We could perhaps add a fixed version of the feature at some point (but not enable it through --strict), but as it's at least somewhat complicated, the core team won't have bandwidth to discuss and review the feature at least in the near term. Feel free to reopen the discussion at a later date, say 6 months from now. Things may be different by then.

@gvanrossum

This comment has been minimized.

Member

gvanrossum commented May 18, 2017

Honestly I strongly believe it's an anti-pattern to do this in Python and I don't want to support a feature (even if contributed). Sorry.

gvanrossum added a commit to gvanrossum/mypy that referenced this issue Oct 5, 2018

Deprecate --strict-boolean
(Putting this in a separate commit since it's more controversial --
since it did work, some people might still be using it, even though
it's been deprecated since May 2017.  See discussion in
python#3195.)

gvanrossum added a commit that referenced this issue Oct 5, 2018

Remove deprecated options, including --strict-boolean (#5740)
These will now all fail with `unrecognized arguments: <flag>`
```
--disallow-any (message points to options replacing it)
-f/--dirty-stubs (ignored)
--use-python-path (exited with error message)
-s/--silent-imports (set ignore_missing_imports=True and follow_imports=skip)
--almost-silent (ses follow_imports=error)
--[no-]fast-parser (ignored)
--strict-boolean (actually did something, see below)
```
Note that `--strict-boolean` might still be somewhat controversial -- some people might still be using it, even though it's been deprecated since May 2017 (and it isn't actually correct).  See discussion in #3195.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment