FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

implement @no_type_check for class defs by lunixbochs · Pull Request #6584 · python/mypy · GitHub

  1. python
  2. mypy

Repository navigation

implement @no_type_check for class defs - #6584

Closed
lunixbochs wants to merge 1 commit into
python:masterfrom
lunixbochs:master
ConversationCommits (1)ChecksFiles changed
Closed

implement @no_type_check for class defs#6584
lunixbochs wants to merge 1 commit into
python:masterfrom
lunixbochs:master

Conversation

lunixbochs commented Mar 22, 2019 •
edited
Loading

Copy link
Copy Markdown

Fixes #607 (which is incorrectly titled).

This adds TypeInfo.is_no_type_check, and enables TypeInfo.fallback_to_any for classes decorated with @no_type_check, which is checked in a couple of places to abort the type checker early and stub method types as Any.

I'm not sure confident about my use of decorator.name in ('no_type_check', 'typing.no_type_check' in semanal.py. I see RefName.fullname = 'typing.no_type_check' was used elsewhere but it wasn't a RefName in the AST during semanal and I didn't look too far into that.

This is my current test:

from typing import no_type_check, Any

@no_type_check
class TestClass:
    bar: {'x': 1} = 1

    def meth1(): pass
    def meth2(a: {'x': 1}) -> {'x': 1}:
        def garbage_inner(a: {'x': 1}):
            pass

    def test(a: int): pass

TestClass().test(1)
TestClass().doesntexist()

class Test(TestClass):
    def test2(self, a: int): pass

t = Test()
t.meth1()
t.doesntexist2()
t.test(1)
t.test2(1)

x: int = TestClass().bar

With @no_type_check, mypy throws a Missing return statement for the nested garbage_inner definition, but suppresses the rest of the type errors for this class and subclass.

Without @no_type_check, we get this nice pile of errors:

repro.py:4: error: Invalid type comment or annotation
repro.py:6: error: Method must have at least one argument
repro.py:7: error: Invalid type comment or annotation
repro.py:8: error: Invalid type comment or annotation
repro.py:11: error: Self argument missing for a non-static method (or an invalid type for self)
repro.py:13: error: Too many arguments for "test" of "TestClass"
repro.py:14: error: "TestClass" has no attribute "doesntexist"
repro.py:21: error: "Test" has no attribute "doesntexist2"
repro.py:22: error: Too many arguments for "test" of "TestClass"

TODO (I'd like advice from core contributors on these):

  • I think I should write in-tree tests for this, but I could use guidance on that.
  • confirm my decorator.name check is correct.
  • is it possible to suppress the no return error?
  • am I missing any additional obvious class type behavior in my test that should be suppressed/stubbed by the decorator?

gvanrossum mentioned this pull request Mar 22, 2019
Open

lunixbochs commented Nov 26, 2019

Copy link
Copy Markdown
Author

Can a maintainer look at this / comment?

ilevkivskyi commented Jan 22, 2020

Copy link
Copy Markdown
Member

Few comments:

  • This is still a draft, please make it non-draft and resolve merge conflicts if you want to get a review
  • Your decorator.name check is wrong, you should check for a RefExpr and its fullname instead.
  • I don't think we need a new flag for this. I think fallback_to_any plus replicating effect of @no_type_check for every method should be enough.
  • Please add tests for this, I think https://github.com/python/mypy/blob/master/test-data/unit/check-classes.test is the best place.

lunixbochs commented Mar 6, 2020 •
edited
Loading

Copy link
Copy Markdown
Author
  • At the point of SemanticAnalyzer.analyze_class, all defn.decorators are still of type NameExpr. How/where can I get RefExpr decorators instead at that point?
  • I used the special flag because I needed to propagate the behavior to semanal3 and analyze_instance_member_access, which fallback_to_any isn't expressive enough to do. I don't see how to propagate the behavior to methods at all at the point where we're just skipping analyze_class.

I'm also now getting fallback can't be filled out until semanal I guess due to skipping analysis with the new semantic analyzer.

msullivan commented Oct 18, 2020

Copy link
Copy Markdown
Collaborator

I'm going to close this because it is against the old semantic analyzer which is totally gone now. Feel free to redo this against the current code base and I'll review it.

msullivan closed this Oct 18, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support @no_type_check for classes

3 participants

Back | FazBrowse Home | New Git URL