Allow instantiation of Type[A], if A is abstract #2853

ilevkivskyi · 2017-02-12T10:55:31Z

Fixes #1843 (It was also necessary to fix few minor things to make this work correctly)

The rules are simple, assuming we have:

class A:
    @abstractmethod
    def m(self) -> None: pass
class C(A):
    def m(self) -> None:
        ...

then

def fun(cls: Type[A]):
    cls() # OK
fun(A) # Error
fun(C) # OK

The same applies to variables:

var: Type[A]
var() # OK
var = A # Error
var = C # OK

Also there is an option for people who want to pass abstract classes around: type aliases, they work as before. For non-abstract A, Type[A] also works as before.

@gvanrossum You could be interested in this.
My intuition why you opened #1843 is when someone writes annotation Type[A] with an abstract A, then most probably one wants a class object that implements a certain protocol, not just inherits from A.

EDIT: as discussed in python/peps#224 this behaviour is good for both protocols and usual ABCs.

elazarg · 2017-02-16T12:57:16Z

#2430 depends on this PR (although the test should be removed there)

gvanrossum

Thanks! I finally got to looking at this. I like the general idea.

gvanrossum · 2017-03-20T23:39:28Z

test-data/unit/check-abstract.test

@@ -157,6 +157,43 @@ class B(A): pass
 B()# E: Cannot instantiate abstract class 'B' with abstract attributes 'f' and 'g'
 [out]

+[case testInstantiationAbstractsInType]
+# flags: --python-version 3.6


I take it this is only because of the var: Type[A] below? If this behavior is supposed to work for all Python versions maybe you can rewrite that part of the test using a function? (Also it's a rather long test; maybe it would be better split up.)

Since recently --python-version flag is not needed. Also I split the test into three smaller and added a test case for @classmethod.

gvanrossum · 2017-03-21T02:34:43Z

mypy/checker.py

@@ -1149,6 +1151,16 @@ def check_assignment(self, lvalue: Lvalue, rvalue: Expression, infer_lvalue_type
                else:
                    rvalue_type = self.check_simple_assignment(lvalue_type, rvalue, lvalue)

+                # Special case


For what, exactly?

I added a detailed comment here.

gvanrossum · 2017-03-21T02:35:24Z

mypy/checker.py

@@ -1149,6 +1151,16 @@ def check_assignment(self, lvalue: Lvalue, rvalue: Expression, infer_lvalue_type
                else:
                    rvalue_type = self.check_simple_assignment(lvalue_type, rvalue, lvalue)

+                # Special case
+                if (
+                    isinstance(rvalue_type, CallableType) and rvalue_type.is_type_obj() and


I'm not keen on this formatting.

gvanrossum · 2017-03-21T02:36:26Z

mypy/checker.py

+                    isinstance(lvalue_type, TypeType) and
+                    isinstance(lvalue_type.item, Instance) and lvalue_type.item.type.is_abstract
+                ):
+                    self.fail("Cannot only assign non-abstract classes"


"Cannot only" -> "Cannot" (I presume).

Oh, sorry, it should be "Can only" (this is probably a worst kind of mistakes).

gvanrossum · 2017-03-21T02:40:19Z

mypy/checkexpr.py

-            if callee.is_concrete_type_obj() and callee.type_object().is_abstract:
+            if (callee.is_type_obj() and callee.type_object().is_abstract
+                    # Exceptions for Type[...] and classmethod first argument
+                    and not callee.from_type_type and not callee.is_classmethod_class):


Maybe also except static methods?

I found a case in our codebase that goes basically like this:

class Logger: @staticmethod def log(a: Type[C]): ... class C: @classmethod def action(cls) -> None: Logger.log(cls) #E: Only non-abstract class can be given where 'Type[C]' is expected @abstractmethod ...

(I'm far from positive that it's because of this line though.)

This code looks reasonable, I added a corresponding exception to check_arg.

gvanrossum · 2017-03-21T02:43:37Z

mypy/checkexpr.py

@@ -840,6 +845,12 @@ def check_arg(self, caller_type: Type, original_caller_type: Type,
            messages.does_not_return_value(caller_type, context)
        elif isinstance(caller_type, DeletedType):
            messages.deleted_as_rvalue(caller_type, context)
+        # Only non-abstract class could be given where Type[...] is expected


could -> can (again :-)

ilevkivskyi · 2017-03-21T09:52:37Z

@gvanrossum
Thank you very much for review!
I applied your comments in new commits.

gvanrossum · 2017-03-21T17:51:44Z

LGTM. We're holding off merging until we've sorted out some issues with recent mypy and typeshed commits.

ilevkivskyi · 2017-03-27T19:07:38Z

@gvanrossum Could this be merged soon?

Sorry for bothering you again, this will be helpful to avoid merge conflicts with my work on protocols.

gvanrossum · 2017-03-27T19:19:01Z

NP. We're finally at a point where we can merge PRs with confidence again.

ilevkivskyi added 4 commits February 12, 2017 00:13

Some ideas

b0ff0cc

Work on corner cases, more tests

71ea62f

Fix minor issues and lint

ec22e4d

Formatting

80c82e7

Merge branch 'master' into abstract-type-ok

f52a3bd

ilevkivskyi mentioned this pull request Mar 13, 2017

PEP 544: Protocols python/peps#224

Merged

gvanrossum reviewed Mar 21, 2017

View reviewed changes

ilevkivskyi added 2 commits March 21, 2017 09:53

Apply review comments

65eab95

Add test fixture; fix formatting

4d2e6fd

gvanrossum approved these changes Mar 21, 2017

View reviewed changes

gvanrossum mentioned this pull request Mar 23, 2017

Type annotation for map with abstract classes as values #3048

Closed

gvanrossum merged commit 72967fd into python:master Mar 27, 2017

elazarg mentioned this pull request Mar 27, 2017

Fix: uninitialized CallableType.is_classmethod_class #2430

Closed

majutsushi mentioned this pull request Mar 24, 2019

Instantiating concrete classes fails check in some cases #6586

Closed

achimnol mentioned this pull request Dec 2, 2019

Inferencing type reference of abstract class inferred from concrete type values #8050

Closed

alessiamarcolini mentioned this pull request Sep 20, 2020

Grooming and cleaning pt3 histolab/histolab#145

Merged

iutlu mentioned this pull request Jul 22, 2024

type[A] cannot be instantiated via partial if A is abstract #17556

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow instantiation of Type[A], if A is abstract #2853

Allow instantiation of Type[A], if A is abstract #2853

ilevkivskyi commented Feb 12, 2017 •

edited

Loading

elazarg commented Feb 16, 2017

gvanrossum left a comment

gvanrossum Mar 20, 2017

ilevkivskyi Mar 21, 2017

gvanrossum Mar 21, 2017

ilevkivskyi Mar 21, 2017

gvanrossum Mar 21, 2017

gvanrossum Mar 21, 2017

ilevkivskyi Mar 21, 2017

gvanrossum Mar 21, 2017

ilevkivskyi Mar 21, 2017

gvanrossum Mar 21, 2017

ilevkivskyi commented Mar 21, 2017

gvanrossum commented Mar 21, 2017

ilevkivskyi commented Mar 27, 2017

gvanrossum commented Mar 27, 2017

Allow instantiation of Type[A], if A is abstract #2853

Allow instantiation of Type[A], if A is abstract #2853

Conversation

ilevkivskyi commented Feb 12, 2017 • edited Loading

elazarg commented Feb 16, 2017

gvanrossum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilevkivskyi commented Mar 21, 2017

gvanrossum commented Mar 21, 2017

ilevkivskyi commented Mar 27, 2017

gvanrossum commented Mar 27, 2017

ilevkivskyi commented Feb 12, 2017 •

edited

Loading