Skip to content

TYP: rename FrameOrSeries to NDFrameT #43752

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 7 commits into from
Sep 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions pandas/_typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@
]
Timezone = Union[str, tzinfo]

# FrameOrSeries is stricter and ensures that the same subclass of NDFrame always is
# used. E.g. `def func(a: FrameOrSeries) -> FrameOrSeries: ...` means that if a
# NDFrameT is stricter and ensures that the same subclass of NDFrame always is
# used. E.g. `def func(a: NDFrameT) -> NDFrameT: ...` means that if a
# Series is passed into a function, a Series is always returned and if a DataFrame is
# passed in, a DataFrame is always returned.
FrameOrSeries = TypeVar("FrameOrSeries", bound="NDFrame")
NDFrameT = TypeVar("NDFrameT", bound="NDFrame")

Axis = Union[str, int]
IndexLabel = Union[Hashable, Sequence[Hashable]]
Expand Down
4 changes: 2 additions & 2 deletions pandas/core/apply.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
AggFuncTypeDict,
AggObjType,
Axis,
FrameOrSeries,
NDFrameT,
)
from pandas.util._decorators import cache_readonly
from pandas.util._exceptions import find_stack_level
Expand Down Expand Up @@ -1120,7 +1120,7 @@ def apply_standard(self) -> DataFrame | Series:
class GroupByApply(Apply):
def __init__(
self,
obj: GroupBy[FrameOrSeries],
obj: GroupBy[NDFrameT],
func: AggFuncType,
args,
kwargs,
Expand Down
6 changes: 3 additions & 3 deletions pandas/core/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
from pandas._typing import (
ArrayLike,
DtypeObj,
FrameOrSeries,
IndexLabel,
NDFrameT,
Shape,
npt,
)
Expand Down Expand Up @@ -181,13 +181,13 @@ class SpecificationError(Exception):
pass


class SelectionMixin(Generic[FrameOrSeries]):
class SelectionMixin(Generic[NDFrameT]):
"""
mixin implementing the selection & aggregation interface on a group-like
object sub-classes need to define: obj, exclusions
"""

obj: FrameOrSeries
obj: NDFrameT
_selection: IndexLabel | None = None
exclusions: frozenset[Hashable]
_internal_names = ["_cache", "__setstate__"]
Expand Down
8 changes: 4 additions & 4 deletions pandas/core/computation/align.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import numpy as np

from pandas._typing import FrameOrSeries
from pandas.errors import PerformanceWarning

from pandas.core.dtypes.generic import (
Expand All @@ -28,14 +27,15 @@
from pandas.core.computation.common import result_type_many

if TYPE_CHECKING:
from pandas.core.generic import NDFrame
from pandas.core.indexes.api import Index


def _align_core_single_unary_op(
term,
) -> tuple[partial | type[FrameOrSeries], dict[str, Index] | None]:
) -> tuple[partial | type[NDFrame], dict[str, Index] | None]:

typ: partial | type[FrameOrSeries]
typ: partial | type[NDFrame]
axes: dict[str, Index] | None = None

if isinstance(term.value, np.ndarray):
Expand All @@ -49,7 +49,7 @@ def _align_core_single_unary_op(


def _zip_axes_from_type(
typ: type[FrameOrSeries], new_axes: Sequence[Index]
typ: type[NDFrame], new_axes: Sequence[Index]
) -> dict[str, Index]:
return {name: new_axes[i] for i, name in enumerate(typ._AXIS_ORDERS)}

Expand Down
8 changes: 4 additions & 4 deletions pandas/core/describe.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import numpy as np

from pandas._libs.tslibs import Timestamp
from pandas._typing import FrameOrSeries
from pandas._typing import NDFrameT
from pandas.util._validators import validate_percentile

from pandas.core.dtypes.common import (
Expand All @@ -45,12 +45,12 @@

def describe_ndframe(
*,
obj: FrameOrSeries,
obj: NDFrameT,
include: str | Sequence[str] | None,
exclude: str | Sequence[str] | None,
datetime_is_numeric: bool,
percentiles: Sequence[float] | np.ndarray | None,
) -> FrameOrSeries:
) -> NDFrameT:
"""Describe series or dataframe.

Called from pandas.core.generic.NDFrame.describe()
Expand Down Expand Up @@ -91,7 +91,7 @@ def describe_ndframe(
)

result = describer.describe(percentiles=percentiles)
return cast(FrameOrSeries, result)
return cast(NDFrameT, result)
Copy link
Member

Choose a reason for hiding this comment

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

There is either a bug in the code or the NDFrameT typevar is not appropriate here...

import pandas as pd
print(pd.__version__)

print(type(pd.DataFrame(columns=["a"]).describe()))

print(type(pd.Series(dtype="object").describe()))

class DataFrameSubClass(pd.DataFrame):
    pass

print(type(DataFrameSubClass(columns=["a"]).describe()))

class SeriesSubClass(pd.Series):
    pass

print(type(SeriesSubClass(dtype="object").describe()))
1.4.0.dev0+768.g7dd22546bc
<class 'pandas.core.frame.DataFrame'>
<class 'pandas.core.series.Series'>
<class 'pandas.core.frame.DataFrame'>
<class 'pandas.core.series.Series'>

although this could be a use case for the other typevar constructor (without bound=..., where the types are not preserved other than the type listed in the TypeVar constructor...

import pandas as pd
from typing import TypeVar

NDFrameT = TypeVar("NDFrameT", bound=pd.core.generic.NDFrame)

FrameOrSeriesT = TypeVar("FrameOrSeriesT", pd.Series, pd.DataFrame)

class DataFrameSubClass(pd.DataFrame):
    pass

class SeriesSubClass(pd.Series):
    pass

def describe(obj: NDFrameT) -> NDFrameT:
    pass

reveal_type(describe(pd.DataFrame()))
reveal_type(describe(pd.Series()))
reveal_type(describe(DataFrameSubClass()))
reveal_type(describe(SeriesSubClass()))

def describe_alt(obj: FrameOrSeriesT) -> FrameOrSeriesT:
    pass

reveal_type(describe_alt(pd.DataFrame()))
reveal_type(describe_alt(pd.Series()))
reveal_type(describe_alt(DataFrameSubClass()))
reveal_type(describe_alt(SeriesSubClass()))
/home/simon/t1.py:17: note: Revealed type is "pandas.core.frame.DataFrame*"
/home/simon/t1.py:18: note: Revealed type is "pandas.core.series.Series*"
/home/simon/t1.py:19: note: Revealed type is "t1.DataFrameSubClass*"
/home/simon/t1.py:20: note: Revealed type is "t1.SeriesSubClass*"
/home/simon/t1.py:25: note: Revealed type is "pandas.core.frame.DataFrame*"
/home/simon/t1.py:26: note: Revealed type is "pandas.core.series.Series*"
/home/simon/t1.py:27: note: Revealed type is "pandas.core.frame.DataFrame*"
/home/simon/t1.py:28: note: Revealed type is "pandas.core.series.Series*"

Copy link
Member Author

Choose a reason for hiding this comment

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

The body of describe_ndframe makes it clear that it does not work with any NDFrame: Series-like is treated as a Series and DataFrame-like is treated as a DataFrame.

The TypeVar is "correct" - replacing print(type()) in your first example with reveal_type:
bar.py:3: note: Revealed type is "pandas.core.frame.DataFrame*"
bar.py:5: note: Revealed type is "pandas.core.series.Series*"
bar.py:12: note: Revealed type is "bar.DataFrameSubClass*"
bar.py:19: note: Revealed type is "bar.SeriesSubClass*"

I think that NDFrameT is probably still the best annotation here.

Copy link
Member Author

Choose a reason for hiding this comment

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

FrameOrSeriesT would be better, until describe is generic enough (if that is a goal)

Copy link
Member Author

Choose a reason for hiding this comment

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

It wouldn't make much of a difference: TypeVar is covariant by default, it would suggest that sub-classes are also handled correctly (which they are not in the case of describe). Could make an invariant TypeVar or leave as-is?



class NDFrameDescriberAbstract(ABC):
Expand Down
Loading