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

Should we encourage abstract types? #12

Closed
gvanrossum opened this issue Oct 16, 2014 · 12 comments
Closed

Should we encourage abstract types? #12

gvanrossum opened this issue Oct 16, 2014 · 12 comments

Comments

@gvanrossum
Copy link
Member

In principle yes, in practice there are issues (long type names, strings and bytes being iterables and sequences).

@JukkaL
Copy link
Contributor

JukkaL commented Oct 16, 2014

I don't think it's a problem if local variables or attributes have concrete types.

Also, function return types should often be concrete, in my opinion, since dynamically typed code can easily make assumptions about the concrete types, and I don't want to make statically typed code unnecessarily restrictive compared to dynamically typed code. MutableSequence, for example, has a smaller interface than list, so this is a non-trivial issue. Since MutableSequence does not define __add__, if a function returns a MutableSequence, the return value cannot be concatenated to a list. Thus if I write a function that always returns a list, I'd probably declare the return type as a list to enable concatenation (and other list-specific operations).

I think the biggest benefits of abstract types are for function argument types. I guess we can safely recommend using abstract types for argument types in public interfaces, at least.

@ambv
Copy link
Contributor

ambv commented Jan 7, 2015

Yes, variables and attributes have concrete types and it's all good. Function return types might want to be concrete or abstract, depending on who defines the interface (see: Protocols). __add__ in lists is a good example to show how this might be an issue. Can mypy discover this? If not, we might want to warn about this subtlety.

@gvanrossum
Copy link
Member Author

Łukasz, I think you can close this issue once you add a sentence to the PEP text.

@flying-sheep
Copy link

hmm, i don’t like non-abstract types here.

what advantages does the existence of List, DIct, Set, and FrozenSet have?

why not just use consistent abstract classes: in shell-expand: {Mutable,}{Sequence,Mapping,Set}

@gvanrossum
Copy link
Member Author

Remember that Python is pretty pragmatic. List has a sort() method, which MutableSequence does not (and should not) have. List+List works, while Sequence+Sequence doesn't. Etc.

@flying-sheep
Copy link

maybe we should add new mixin methods that address this?

Sequence could define __add__, and MutableSequence could define sort (should be no problem as it already does define reverse)

alternatively, we could add new generic abstract classes that define the functionality not covered by the current ABCs, but not bound to concrete builtin types.

@abarnert
Copy link

[1] + (2, 3) is a TypeError. I don't think you're suggesting to change that. (Otherwise, ['abc'] + 'def' would succeed, presumably returning ['abc', 'd', 'e', 'f'], and I doubt you want that?)

Of course you could define Sequence.__add__ requires the other parameter to be covariant with self instead of contravariant (although I don't think there's any way to write that in the current syntax, but it's actually a general problem with __add__ and other operator methods that are effectively multiple dispatch…). But that still wouldn't help. If a user writes f() + [1, 2], and f is only defined to return MutableSequence instead of list, it will fail the check.

Also, a mixin method for sort would be a pretty ugly thing, because anyone calling x.sort() is presumably expecting an in-place sort, but the only way MutableSequence.sort could be implemented i something like self[:] = sorted(self).

So, I think it's reasonable that methods should be able to specify concrete return types like list, or List[int].

@abarnert
Copy link

I have a different worry here.

Everyone seems to agree with this:

I think the biggest benefits of abstract types are for function argument types. I guess we can safely recommend using abstract types for argument types in public interfaces, at least.

But every iteration of the proposal so far has left that out. If the docs don't recommend using abstract types for arguments in public interfaces, the concrete types will be an attractive nuisance; people will write functions that demand a List when they really only need a Sequence or Iterable.

And really, I think that's the core worry that flying-sheep has, even if he hasn't realized it yet.

@flying-sheep
Copy link

[1] + (2, 3) is a TypeError

i understand your concerns except MutableSequence.sort. As said: MutableSequence.reverse exists, and i don’t see a difference here. both modify the sequence by changing the order of its elements.

Another thing to do would be to specify something like ConcatenableSequence[Self] or ConcatenableSequence[Sequence], where Self would mean “the concrete type of this sequence”. problem here is that noone will use this, and therefore the linter will warn everyone who uses + with a method return value, even if that return value is a buitin list.

I have a different worry here…

that’s my main worry as well. sorry for not saying that: List is of course useful for local variables, and returning a List is also fine for things like the mentioned __add__ concatenation, but list sucks for function arguments, as they will almost always only expect a Sequence or at most a MutableSequence.

@gvanrossum
Copy link
Member Author

This whole issue exists to remind us that the PEP should have some language encouraging abstract types over concrete types. (And for "that" a PR would be welcome.) However that doesn't mean we should not define concrete types. Nothing is gained if people decide to use Any or plain list because they can't write List[AnyStr].

@JukkaL
Copy link
Contributor

JukkaL commented Jan 17, 2015

See #48 for another angle to this issue.

@gvanrossum
Copy link
Member Author

I've added some words to the PEP. Closing.

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

No branches or pull requests

5 participants