`concat` of types `Iterable[Any]` cause Pyright to think subsequent code is unreachable because of recent stubs change

[torext]

Describe the bug
In the following code the last line is wrongly considered to be unreachable by Pyright:

from typing import Any

import pandas as pd


def generate_series() -> Any:
    return pd.Series([1, 2, 3])


df = pd.concat({k: generate_series() for k in range(10)}, axis=1)

print("Unreachable?!")

This is caused by the recent change #858 adding a stub returning Never. This is presumably a misuse of Never or the stub as a whole should be moved to the very end so that concat on types Iterable[Any] can first match a stub with valid return.

To Reproduce
See microsoft/pylance-release#5631 and microsoft/pylance-release#5630 for issues reported on the Pylance repo.

Please complete the following information:

• OS: Linux
• OS Version 5.15.0-46-generic
• Python 3.11.6
• Pylance v2024.3.1
• version of installed pandas-stubs: the one shipped with above Pylance version

[Dr-Irv]

Interesting. We were trying to prevent Iterable[None], but in the process blocked Iterable[Any].

Asking @twoertwein to look at this.

[debonte]

We were trying to prevent Iterable[None]...

Note that returning Never from an overload means that with that combination of arguments, the function will never return (ex. it always raises an exception, or loops infinitely). It doesn't mean, "this combination of argument values is illegal".

[Dr-Irv]

We were trying to prevent Iterable[None]...

Note that returning Never from an overload means that with that combination of arguments, the function will never return (ex. it always raises an exception, or loops infinitely). It doesn't mean, "this combination of argument values is illegal".

In this case, if you do pd.concat([None]), pandas does raise an exception. So our meaning is correct here.

If we move that overload down, then it will never match, because pd.concat([pd.Series(), None]) is valid, which matches the second overload, and we don't have a way to say "you can't pass an iterable of just None, but mixing None with Series or DataFrame is OK"

The main issue here is the "wildcard" nature of Any . It's unclear to me whether we should be supporting the use of parameters that are typed as Any by users and passed into methods declared in the stubs, because that defeats the purpose of type checking, IMHO.

[torext]

I'd rather say the main issue here is that you're trying to support a case (i.e. all None args) that is very unlikely to occur in practice in a way that needs to be caught by a static type checker and where an appropriate runtime error (which is already there) doesn't suffice.

Instead when one omits type annotations from function returns it seems that many type checkers will implicitly consider the return type to be Any, unless otherwise inferable. The example I gave above is purely artifical to highlight the problem, but I've encountered the problem in practice with very generic code that doesn't have type annotations.

Given type annotations are purely optional, I'd say no combination of type checkers and stubs should ever declare code unreachable when no type annotations are present and no obvious inference of unreachability can be made (like e.g. if False:...).

[debonte]

It's unclear to me whether we should be supporting the use of parameters that are typed as Any by users and passed into methods declared in the stubs, because that defeats the purpose of type checking, IMHO.

This scenario may be more interesting than explicit Any:

pd.concat([])
print("Unreachable?!")

FWIW, Pylance had two issues filed on this (one with 5 up votes) within about 12 hours of shipping this change.

[Dr-Irv]

I'd rather say the main issue here is that you're trying to support a case (i.e. all None args) that is very unlikely to occur in practice in a way that needs to be caught by a static type checker and where an appropriate runtime error (which is already there) doesn't suffice.

I think that is debatable. If the type checker can find the error before getting to runtime, that's a good thing.

Instead when one omits type annotations from function returns it seems that many type checkers will implicitly consider the return type to be Any, unless otherwise inferable. The example I gave above is purely artifical to highlight the problem, but I've encountered the problem in practice with very generic code that doesn't have type annotations.

Yes, that's true, but the question is whether our goal is to support well-typed code that calls pandas, or untyped code that calls pandas. We've chosen the former from a philosophical point of view.

Given type annotations are purely optional, I'd say no combination of type checkers and stubs should ever declare code unreachable when no type annotations are present and no obvious inference of unreachability can be made (like e.g. if False:...).

The unreachable conclusion made by pylance within VSCode is their choice. If you run your code through mypy or command line pyright, then it passes fine without any errors.

The python typing page says the following about Never:

This can be used to define a function that should never be called, or a function that never returns:

That's exactly what we used it for.

[Dr-Irv]

It's unclear to me whether we should be supporting the use of parameters that are typed as Any by users and passed into methods declared in the stubs, because that defeats the purpose of type checking, IMHO.

This scenario may be more interesting than explicit Any:

pd.concat([])
print("Unreachable?!")

And that code IS unreachable!! If you try to execute pd.concat([]), you will get an exception!

FWIW, Pylance had two issues filed on this (one with 5 up votes) within about 12 hours of shipping this change.

[debonte]

And that code IS unreachable!! If you try to execute pd.concat([]), you will get an exception!

Ah, ok. I thought the exception was only thrown if all of the collection's (one or more) elements were None. I didn't realize it was also thrown for empty collections.

This can be used to define a function that should never be called, or a function that never returns:

That's exactly what we used it for.

I believe the "never be called" case is talking about using Never to annotate function parameters, not function return types.

[torext]

Yes, that's true, but the question is whether our goal is to support well-typed code that calls pandas, or untyped code that calls pandas. We've chosen the former from a philosophical point of view.

Apologies for my ignorance, but shouldn't untyped code be considered the default case? Unless these stubs are widely used by different IDEs and type checkers then what is the point of making them? And unless the latter don't behave sanely and usefuly for the base-case of untyped code, then what's the point of those in turn?

My point being, again, it seems to me that you are prioritising here a very particular edge case purely for the sake of prioritising that niche case. I on the other hand have work to do, and I need a solid cross-OS IDE for various languages and in particular Python.

Apologies for the likely palpable frustration on my side, but I hope I'm making it clear where I'm coming from on this. You said you guys made a philosophical choice, I'm instead urging you to consider pragmatism.

[Dr-Irv]

I believe the "never be called" case is talking about using Never to annotate function parameters, not function return types.

That's unclear. See https://mypy.readthedocs.io/en/latest/more_types.html#the-noreturn-type with respect to NoReturn. But then the current typing docs at https://docs.python.org/3/library/typing.html say about Never "This can be used to define a function that should never be called, or a function that never returns:" and "NoReturn can also be used as a bottom type, a type that has no values. Starting in Python 3.11, the Never type should be used for this concept instead. Type checkers should treat the two equivalently."

So our interpretation of Never is correct - it means a function that never returns, i.e., if you pass those parameters, you don't get a return - it can just die via sys.exit() or raise an Exception

[Dr-Irv]

Apologies for my ignorance, but shouldn't untyped code be considered the default case?

No!!!

When my team had a large untyped code, and we then converted it to have types, the static typing checks actually found bugs that we didn't know existed.

Here's a really simple example:

s = pd.Series([3.4, 2.3])
r: Any = 3.2
s.round(r)

This code will fail, because the argument for round() should be an int. But because r is declared to be Any, the type checker says it is valid code.

If you remove the Any declaration, then the type checker sees that you passed an incorrect argument.

The problem with using Any is that it matches anything. The advantage of using type checking is that it allows you to find errors in your code prior to runtime. But you only get that advantage if you start introducing types into your code. Using Any doesn't allow you to obtain the benefits of type checking.

[twoertwein]

@torext I cannot reproduce your error on main with the latest pyright (even when adding # pyright: strict) - it does not complain about unreachable code! It, however, reports Type of "concat" is partially unknown (reportUnknownMemberType).