Skip to content

feat(index): append #1282

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.

Sign up for GitHub

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
Dr-Irv merged 11 commits into from
Jul 30, 2025
Merged

feat(index): append #1282

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
390e5d9
feat(index): append
cmp0xff Jul 18, 2025
3a34057
fix(comment): https://github.com/pandas-dev/pandas-stubs/pull/1282#di…
cmp0xff Jul 18, 2025
45e0ad3
fix(comment): https://github.com/pandas-dev/pandas-stubs/pull/1282/fi…
cmp0xff Jul 18, 2025
6c8ba76
fix(pyrefly): ignore
cmp0xff Jul 18, 2025
3844062
fix(comment): https://github.com/pandas-dev/pandas-stubs/pull/1282#di…
cmp0xff Jul 19, 2025
ac0857b
fix(comment): https://github.com/pandas-dev/pandas-stubs/pull/1282#di…
cmp0xff Jul 29, 2025
67e6bde
fix(comment): https://github.com/pandas-dev/pandas-stubs/pull/1282#di…
cmp0xff Jul 29, 2025
fa3c401
fix: Sequence[Index] instead of Sequence
cmp0xff Jul 29, 2025
d7a538d
fix(comment): avoid union types https://github.com/pandas-dev/pandas-…
cmp0xff Jul 29, 2025
fe5f1a9
chore: https://github.com/pandas-dev/pandas-stubs/pull/1282#discussio…
cmp0xff Jul 30, 2025
1c75df6
fix: constraint instead of bound to get rid of Any https://github.com…
cmp0xff Jul 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix(comment): #1282 (comment)
  • Loading branch information
@cmp0xff
cmp0xff committed Jul 18, 2025
commit 3a3405797429eb262949b6b9d3c7e6d6a0d301bd
2 changes: 0 additions & 2 deletions pandas-stubs/core/indexes/base.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,6 @@ class Index(IndexOpsMixin[S1]):
@overload
def __getitem__(self, idx: int | tuple[np_ndarray_anyint, ...]) -> S1: ...
@overload
def append(self, other: Index[Never]) -> Index: ...
@overload
def append(self, other: Index[S1] | Sequence[Index[S1]]) -> Index[S1]: ...
Copy link
Member

@twoertwein twoertwein Jul 18, 2025

Choose a reason for hiding this comment

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

it might be nicer here to have a second S1 here as the resulting Index can contain a mix of different types.

def append(self, other: Index[S2] | Sequence[Index[S2]]) -> Index[S1 | S2]: ...

Copy link
Contributor Author

@cmp0xff cmp0xff Jul 18, 2025

Choose a reason for hiding this comment

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

45e0ad3, but this one works less well.

  • mypy is not happy with Index[int].append(Index[int | str]) and gives Index[Any]
  • pyright is not happy with Index[int | str].append([Index[int], Index[str]]) and gives Index[int | Any]. In particular, the typing for [Index[int], Index[str]] seems to be list[Index[int] | Index[str]], instead of list[Index[int | str]].

Copy link
Member

@twoertwein twoertwein Jul 18, 2025

Choose a reason for hiding this comment

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

  • mypy is not happy with Index[int].append(Index[int | str]) and gives Index[Any]
  • pyright is not happy with Index[int | str].append([Index[int], Index[str]]) and gives Index[int | Any]. In particular, the typing for [Index[int], Index[str]] seems to be list[Index[int] | Index[str]], instead of list[Index[int | str]].

While that is annoying for testing on the CI, I think that is the safer choice for user: rather expect a wider type that includes Any than suggesting it is a narrower type. This needs input from @Dr-Irv.

If S1 and S2 were covariant, it seems to work for at least pyright in a simple toy example (but they are invariant)

from __future__ import annotations
from typing import TypeVar, reveal_type, Generic, Sequence

S1 = TypeVar("S1", bound=int | str, covariant=True)
S2 = TypeVar("S2", bound=int | str, covariant=True)

class Index(Generic[S1]):
    def __init__(self, data: list[S1]) -> None: ...

    def append(self: Index[S1], other: Sequence[Index[S2]]) -> Index[S1 | S2]: ...

strings = Index(["a"])
reveal_type(strings)
ints = Index([1])
reveal_type(ints)

reveal_type(strings.append([ints]))
reveal_type(ints.append([strings]))

string_ints = Index(["a", 1])
reveal_type(string_ints)
reveal_type(string_ints.append([ints]))
reveal_type(strings.append([string_ints]))

reveal_type(strings.append([ints]))
reveal_type(strings.append([strings, ints]))

reveal_type(strings.append([ints, strings, string_ints]))

Copy link
Contributor Author

@cmp0xff cmp0xff Jul 19, 2025

Choose a reason for hiding this comment

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

If S1 and S2 were covariant, it seems to work for at least pyright in a simple toy example (but they are invariant)

from __future__ import annotations
from typing import TypeVar, reveal_type, Generic, Sequence

S1 = TypeVar("S1", bound=int | str, covariant=True)
S2 = TypeVar("S2", bound=int | str, covariant=True)

class Index(Generic[S1]):
    def __init__(self, data: list[S1]) -> None: ...

    def append(self: Index[S1], other: Sequence[Index[S2]]) -> Index[S1 | S2]: ...

Hi, I am new to covariance / contravariance, but I read PEP484 (covariance-and-contravariance) and it says covariant is for classes, not for functions, where the latter case is prohibited. In your example, S1 is find, but not S2. Could you help me and explain a bit? Thanks.

B_co = TypeVar('B_co', covariant=True)

def bad_func(x: B_co) -> B_co:  # Flagged as error by a type checker
    ...

Copy link
Member

@twoertwein twoertwein Jul 19, 2025

Choose a reason for hiding this comment

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

We can't change S1 to be covariant. While the following is not exactly what we like to have, it is probably the closest we can get (but it doesn't work with mypy, unless the caller casts).

from __future__ import annotations
from typing import TypeVar, reveal_type, Generic, Sequence

S1 = TypeVar("S1", bound=int | str)
IndexT = TypeVar("IndexT", bound="Index")


class Index(Generic[S1]):
    def __init__(self, data: list[S1]) -> None: ...

    def append(self: Index[S1], other: Sequence[IndexT]) -> Index[S1] | IndexT: ...


strings = Index(["a"])
reveal_type(strings)
ints = Index([1])
reveal_type(ints)

reveal_type(strings.append([ints]))
reveal_type(ints.append([strings]))

string_ints = Index(["a", 1])
reveal_type(string_ints)
reveal_type(string_ints.append([ints]))
reveal_type(strings.append([string_ints]))

reveal_type(strings.append([ints]))
reveal_type(strings.append([strings, ints]))

reveal_type(strings.append([ints, strings, string_ints]))

Copy link
Contributor Author

@cmp0xff cmp0xff Jul 19, 2025

Choose a reason for hiding this comment

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

Hi, I ran the script myself. In the most complicated case, I see Index[int | str] | Index[int] | Index[str]. To be honest, as a user I would rather see Index[Unknown], because it's simpler, and in both cases I would probably still need a manual cast. Nevertheless, 3844062

@overload
def append(self, other: Index | Sequence) -> Index: ...
Expand Down