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

Consider allowing assertIsNotNone() to constitute a None check #2007

Closed
jparise opened this issue Jun 18, 2021 · 2 comments
Closed

Consider allowing assertIsNotNone() to constitute a None check #2007

jparise opened this issue Jun 18, 2021 · 2 comments
Labels
enhancement request New feature or request

Comments

@jparise
Copy link

jparise commented Jun 18, 2021

Is your feature request related to a problem? Please describe.

It's common for unit tests (based on unittest) to assert that an object is non-None and then verify some of its attributes.

def test_object(self):
    obj = load_object_by_name('name')  # type: Optional[Object]
    self.assertIsNotNone(obj)
    self.assertEqual('name', obj.name)

Pyright will flag that last line for reportOptionalMemberAccess, even though we've performed an (implied) "is not None" check via assertIsNotNone.

There are three workarounds:

Somewhat redundant

def test_object(self):
    obj = load_object_by_name('name')  # type: Optional[Object]
    self.assertIsNotNone(obj)
    if obj is not None:
        self.assertEqual('name', obj.name)

Very redundant

def test_object(self):
    obj = load_object_by_name('name')  # type: Optional[Object]
    self.assertIsNotNone(obj)
    assert is not None
    self.assertEqual('name', obj.name)

Tricky but concise

def test_object(self):
    obj = load_object_by_name('name')  # type: Optional[Object]
    self.assertIsNotNone(obj)
    self.assertEqual('name', obj and obj.name)

Describe the solution you'd like

Because unittest is part of the standard library, it would be convenient if calling assertIsNotNone (and assertIsNone) would be constitute a None check from the type checker's perspective.

Additional context

It might be too magical to apply special treatment to these methods. Perhaps there should (eventually) be a generalized typing facility that assertIsNone/assertIsNotNone could adopt, a la TypeGuard.

@erictraut
Copy link
Collaborator

We generally don't hard code any knowledge of specific functions or methods. It's just not a scalable approach to type checking. Type information should come from the type stubs, even for stdlib methods.

In my code, I've worked around this by switching from assertIsNotNone to assert is not None. Another option is to disable the reportOptionalMemberAccess diagnostic check in your test files using a # pyright: reportOptionalMemberAccess=false comment at the top of the file.

We have made exceptions to the above principle in very rare cases but only after we received significant signal from pyright/pylance users. I'm going to close this issue for now, but we can consider reopening it in the future if it receives a bunch of upvotes or additional requests.

@1kastner
Copy link

Well, this issue arises at different corners of GitHub, see also python/typeshed#8583 and the issues mentioned therein. People are still in need of this feature :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement request New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants