Skip to content

Type[C] #1569

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

Merged
merged 23 commits into from
Jun 8, 2016
Merged

Type[C] #1569

merged 23 commits into from
Jun 8, 2016

Conversation

gvanrossum
Copy link
Member

TBD

# Translate constructor fn back to class.
if isinstance(type, CallableType) and type.is_type_obj():
types[i] = type = type.ret_type

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is right, though somehow it never made a difference in my tests. The value C (the class) is not the same sort of thing as an instance of the class C.

@rwbarton
Copy link
Contributor

Do you plan to add support for invoking a class method on a value of type Type[C]?

@gvanrossum
Copy link
Member Author

Of course; I hope that already works.

--Guido (mobile)
On May 25, 2016 10:03 AM, "Reid Barton" [email protected] wrote:

Do you plan to add support for invoking a class method on a value of type
Type[C]?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#1569 (comment)

@gvanrossum
Copy link
Member Author

gvanrossum commented May 25, 2016 via email

@gvanrossum
Copy link
Member Author

gvanrossum commented May 25, 2016 via email

Guido van Rossum added 10 commits June 6, 2016 15:28
The TypeC tests no pass except for one ProUser* that should be
ProUser; but something's seriously wrong because many other tests now
fail.
- Add tests for Type[Any] and plain Type, and fix a bug that found.
- Respond to easy review feedback (adding comments/docstrings mostly).
We only support two special cases (hopefully the most common ones):

- Type[<concrete_class>]
- Type[<type_variable_with_concrete_class_upper_bound>]
(This happens when type checking a call, not during semantic analysis.)
@gvanrossum
Copy link
Member Author

Another look? I've addressed all XXX comments, and added more tests. Would like to get this in 0.4.2.

@gvanrossum
Copy link
Member Author

Ignore AppVeyor. I don't know why it keeps doing this even though I removed appveyor.yml.

class User: pass
class ProUser(User): pass
def new_user(user_class: Type[User]) -> User:
return user_class()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Test calling with invalid arguments to __init__. Also test a class with non-default __init__ and call constructor with both valid and invalid arguments.

@gvanrossum
Copy link
Member Author

Please ignore the AppVeyor build, as usual.

@gvanrossum
Copy link
Member Author

Addressed all remaining comments.

elif isinstance(self.s, Instance) and self.s.type.fullname() == 'builtins.type':
return self.s
else:
return AnyType()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't return AnyType here -- instead, use self.default(self.s), which usually returns object.

@JukkaL
Copy link
Collaborator

JukkaL commented Jun 7, 2016

Looks pretty good now! Here are a few additional things to test:

  • More tests for Type[Any]. I think we agreed that it shouldn't be possible to access arbitrary attributes, but this should be callable with arbitrary arguments. It's worth testing these.
  • What if C is a named tuple in Type[C]? I assume that it hits the regular tuple code path but it's worth adding a test case, as named tuples sometimes behave badly.
  • I think there isn't a test for calling Type[C] if C has an overloaded __init__. Worth testing this, as overloaded methods occasionally cause trouble.
  • Test a case where we use join during type inference, such as expression [ta, tc] where type of ta is Type[A] and type of tb is Type[B], and B is a subclass of A.

These are optional but if you don't implement these consider creating issues:

  • Should Type[C] be a subtype of Callable[..] in case the signature is compatible?
  • If yes, we should also infer constraints for a Callable template and TypeType actual.
  • If yes, join and meet will likely also be affected.

@gvanrossum
Copy link
Member Author

Pushed solutions for the first 4 bullets.

@gvanrossum
Copy link
Member Author

gvanrossum commented Jun 7, 2016

Looks like Type[C] is indeed a subtype of Callable[[], C]:

class C: pass
T = Type[C]
def f(a: Callable[[], C]) -> None: pass
f(C)
f(T)

has no errors. I can write a more elaborate test after lunch.

UPDATE: But I'm not sure what the last two bullets imply then.
UPDATE2: Filed #1670 for that, left this PR unchanged.

@JukkaL JukkaL merged commit 8555adb into master Jun 8, 2016
@gvanrossum gvanrossum deleted the typec branch June 8, 2016 17:10
ilevkivskyi pushed a commit that referenced this pull request Aug 2, 2018
Fix #3631 : looks like it was an explicit decision, made before we had self types.

The code modified in this PR was introduced at #1569. There are no tests for the current behavior, and it seems like we actually want to allow it.
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.

3 participants