This approach would have been my instinct.

Since the docs for .files suggests to use always_iterable, I'd prefer to follow that recommendation. I'll explore that option and then decide how to proceed.

oh. I see this change combines two changes (adding annotations and fixing the bug for #344). These two concerns should be separate. I recommend to remove these bugfixes from this PR so that it focuses on the type annotations.

I already separated each functional change into separate atomic commits. They cannot be removed from this PR, because then it won’t pass mypy—the point of mypy is to find bugs like this!

I had to remove always_iterable in favor of or [] because the behavior of always_iterable is too dynamic to be correctly annotated in Python’s type system. The behavior where it converts a non-iterable input to a one-item iterator cannot be expressed; there’s an Iterable constraint but not a “non-iterable” constraint. Users who don’t need types are still free to use always_iterable, of course.

(Again, I’ve done this in a separate commit.)

I had to remove always_iterable...

That's a shame and feels like a defect or weakness in the typing system. I'd like to see a solution that doesn't require changing the implementation. The always_iterable idiom is a simple yet powerful abstraction and replaces with a single concise expression the common idiom of:

def block(param):
  if param is None:
    param = []
  if isinstance(param, (str, bytes)):
    param = [param]
  # ... do stuff with iterable of param

If the type system can't support that level of dynamism, then the type system should be avoided until it can.

If that were all always_iterable did, it would be completely fine:

from typing import Iterable, Iterator, Optional, TypeVar

T = TypeVar("T")

def always_iterable(obj: Optional[Iterable[T]]) -> Iterator[T]:
    if obj is None:
        return iter(())

    if isinstance(obj, (str, bytes)):
        return iter((obj,))

    return iter(obj)

(mypy playground)

But the current version has two extra features that interfere with static typing:

def always_iterable(obj, base_type=(str, bytes)):
    if obj is None:
        return iter(())

    if (base_type is not None) and isinstance(obj, base_type):
        return iter((obj,))

    try:
        return iter(obj)
    except TypeError:
        return iter((obj,))

Firstly, a non-default base_type might change the behavior so that obj: Optional[Iterable[T]] does not necessarily imply always_iterable(obj): Iterator[T].

Secondly, the TypeError handler means that one should also have always_iterable(obj): Iterator[T] when obj: T—but only if T is not itself iterable. That extra non-iterable condition is not expressible in mypy, for the fundamental reason that it would break the Liskov substitution principle (e.g. every type is supposed to be a subtype of object, which is non-iterable). It’s also a fragile condition to rely on even when no annotations are involved (what if T is updated in the future to become iterable?).

I see. I had simplified the summary of always_iterable to focus on the implementation for how it's used/recommended here. You're right that the implementation also addresses the concern of transforming a single item to an iterator over one item.

It solves the common pattern where a function accepts both a single item or an iterable of items but intends to iterate over them. For example consider this change, where always_iterable transforms the branchy logic into a simple dict comprehension.

That extra non-iterable condition is not expressible in mypy.

That seems like a limitation of mypy. It seems to me that it should be possible to express any valid Python in mypy, especially something as simple as accepting an iterable or non-iterable. Is it possible the level of expressiveness you're trying to achieve is too great? I notice that always_iterable does already have a type specification. Would that spec suffice or is it too generic for strict mode?

It’s also a fragile condition to rely on

I think the same could be said for or [], which is also fragile based on the value of ob.__bool__. If an object were to return False for __bool__ but not implement iterable, it also would fail. I've found that for all (most?) practical cases, iterability is stable for the uses of any particular function, despite being unreliable in the general case.

That seems like a limitation of mypy.

Yes. But there’s a good reason for this limitation, since it’s a consequence of the more important Liskov substitution principle: wherever an expression of type T is expected, it should be type-correct to provide a value whose type is a subclass of T.

Remember, every non-iterable type might be subclassed by an iterable type (this is not an uncommon corner case: every iterable type is a subclass of object). So if there were a hypothetical syntax that purported to guarantee that always_iterable(obj): T when obj: T and T is non-iterable, one could assign an instance of an iterable subclass U to obj: T and get back a result from always_iterable(obj) that breaks this guarantee.

A simpler way to say this is that there’s no such thing as a non-iterable type, provided that one recognizes a type to implicitly include all of its subclasses. Giving up that provision would have unpleasant ripple effects on every other aspect of the type system (I think you would basically need to augment each type annotation with a flag saying whether or not a subclass is allowed).

I notice that always_iterable does already have a type specification.

Any is never a type specification; it just disables type checking for that value and every value derived from it. (See the mypy documentation and avoid the Any type.) Given that type checking is finding real bugs, we should be very hesitant to disable it for the sake of stylistic preferences.

There will always be some cases where appeasing the type checker rather than disabling it will mean writing code in a different way than you might have otherwise. No static type system can be powerful enough to capture all the ways a language as dynamic as Python can be used. I acknowledge that this is unfortunate. But I hope you’ll find as I have that the benefits of full type checking outweigh these inconveniences.

I think the same could be said for or [], which is also fragile based on the value of ob.__bool__.

It’s conventional in Python, to the point of being enshrined in PEP 8, that testing a sequence as a boolean is an idiomatic check for emptiness. But let’s consider all the corner cases:

• A sequence that’s truthy but empty is fine since the result of or [] is still empty.
• A sequence that’s falsy but nonempty would be treated as empty—given the PEP 8 idiom, if you write such a sequence, you should have expected that some users will treat it that way—but this doesn’t break type safety.
• There’s no value that’s None but truthy, since NoneType cannot be subclassed.

If that doesn’t satisfy you, here are some alternatives that type check:

def _top_level_inferred(dist: Distribution) -> Set[str]:
    return {
        f.parts[0] if len(f.parts) > 1 else f.with_suffix('').name
        for f in ([] if dist.files is None else dist.files)
        if f.suffix == ".py"
    }

def _top_level_inferred(dist: Distribution) -> Set[str]:
    if dist.files is None:
        return set()
    return {
        f.parts[0] if len(f.parts) > 1 else f.with_suffix('').name
        for f in dist.files
        if f.suffix == ".py"
    }

Perhaps the separate if statement is actually the clearest?