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

Change final classes without __bool__ method to always be True #12187

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

BolunThompson
Copy link

@BolunThompson BolunThompson commented Feb 16, 2022

Description

Closes #12158

Changes it so that final classes without a __bool__ or __len__ method are always True.

Test Plan

Tests are added to test/testtypes.py

@BolunThompson BolunThompson changed the title Final class without bool Change final classes without bool to be True Feb 16, 2022
@BolunThompson BolunThompson changed the title Change final classes without bool to be True Change final classes without __boo__ method to always be True Feb 16, 2022
@BolunThompson BolunThompson changed the title Change final classes without __boo__ method to always be True Change final classes without __bool__ method to always be True Feb 16, 2022
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

rich (https://github.com/willmcgugan/rich)
+ rich/spinner.py:83: error: Statement is unreachable

black (https://github.com/psf/black)
+ src/blib2to3/pgen2/tokenize.py:603:29: error: Right operand of "or" is never evaluated

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

Please, also take a look at:

  • rich/spinner.py:83: error: Statement is unreachable

https://github.com/Textualize/rich/blob/8c3e6be424f36522cbb3c4773e58a01f5f39761f/rich/spinner.py#L82-L83

It looks like a false positive. Because not self.text is possible when self.text == ''

@@ -451,6 +459,13 @@ def test_false_only_of_union(self) -> None:
assert fo.items[0].can_be_false
assert fo.items[1] is tup_type

def test_false_only_of_truthy_type_is_uninhabited(self) -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it will be more useful to add tests to some check-*.test file.

I think that these ones can be removed.

@JelleZijlstra
Copy link
Member

The Black change is also a false positive. It's checking a member of this dict: https://github.com/psf/black/blob/main/src/blib2to3/pgen2/tokenize.py#L159

But the members of that dict can be None, so it's legitimate to look at endprogs[some_key].

@BolunThompson
Copy link
Author

BolunThompson commented Jun 6, 2022

I'm too busy to work on this now, so anybody can finish this. If I remember correctly one of the errors related to my PR was not handling Union types correctly; I don't remember the cause of the other error.

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.

(🎁) final class without a __bool__ or __len__ method can never test false
3 participants