Skip to content

CLN: small cleanups #48836

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
merged 1 commit into from
Sep 29, 2022
Merged

CLN: small cleanups #48836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion pandas/_testing/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ def box_expected(expected, box_cls, transpose: bool = True):
# pd.array would return an IntegerArray
expected = PandasArray(np.asarray(expected._values))
else:
expected = pd.array(expected)
expected = pd.array(expected, copy=False)
elif box_cls is Index:
expected = Index._with_infer(expected)
elif box_cls is Series:
Expand Down
18 changes: 7 additions & 11 deletions pandas/core/arrays/datetimelike.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1259,8 +1259,8 @@ def _add_timedeltalike_scalar(self, other):
return type(self)._simple_new(new_values, dtype=self.dtype)

# PeriodArray overrides, so we only get here with DTA/TDA
# error: "DatetimeLikeArrayMixin" has no attribute "_reso"
inc = delta_to_nanoseconds(other, reso=self._reso) # type: ignore[attr-defined]
self = cast("DatetimeArray | TimedeltaArray", self)
inc = delta_to_nanoseconds(other, reso=self._reso)

new_values = checked_add_with_arr(self.asi8, inc, arr_mask=self._isnan)
new_values = new_values.view(self._ndarray.dtype)
Expand All @@ -1270,10 +1270,7 @@ def _add_timedeltalike_scalar(self, other):
# adding a scalar preserves freq
new_freq = self.freq

# error: Unexpected keyword argument "freq" for "_simple_new" of "NDArrayBacked"
return type(self)._simple_new( # type: ignore[call-arg]
new_values, dtype=self.dtype, freq=new_freq
)
return type(self)._simple_new(new_values, dtype=self.dtype, freq=new_freq)

def _add_timedelta_arraylike(
self, other: TimedeltaArray | npt.NDArray[np.timedelta64]
Expand Down Expand Up @@ -2217,11 +2214,11 @@ def validate_periods(periods: None) -> None:


@overload
def validate_periods(periods: float) -> int:
def validate_periods(periods: int | float) -> int:
...


def validate_periods(periods: float | None) -> int | None:
def validate_periods(periods: int | float | None) -> int | None:
Copy link
Member

Choose a reason for hiding this comment

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

@twoertwein so the typing simplification(?) that converted int | float -> float isn't an enforced typing check?

Copy link
Member

Choose a reason for hiding this comment

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

flake8-pyi does that check but only for pyi files not py files.

More generally, we have many Unions that contain redundant classes, for example Scalar contains datetime and pd.Timestamp but I think pd.Timestamp inherits from datetime. That is not a problem (also type checkers might need some more time) but it is not good style.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should I revert this? i find it clearer, but not a big deal.

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't mind ignoring flake8-pyi's Y041 when we run flake8-pyi: then py and pyi files are more consistent and there is also some push back against it python/mypy#13760 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

btw in a world where float is equivalent to int | float, how would i express "specifically a python float object and not an integer object"

Copy link
Member

Choose a reason for hiding this comment

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

In the type checkers world, you can't: https://peps.python.org/pep-0484/#the-numeric-tower

Rather than requiring that users write import numbers and then use numbers.Float etc., this PEP proposes a straightforward shortcut that is almost as effective: when an argument is annotated as having type float, an argument of type int is acceptable; similar, for an argument annotated as having type complex, arguments of type float or int are acceptable. This does not handle classes implementing the corresponding ABCs or the fractions.Fraction class, but we believe those use cases are exceedingly rare.

But I wonder how python and its type system would have evolved if they started together - we hopefully wouldn't have this mess of type checkers/PEPs dictating that int is treated as a subclass of float but the implementation doesn't do that.

Copy link
Member Author

Choose a reason for hiding this comment

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

that seems weird given that int isnt a subclass of float. anyway, shouldnt be relevant for this PR

"""
If a `periods` argument is passed to the Datetime/Timedelta Array/Index
constructor, cast it to an integer.
Expand All @@ -2244,9 +2241,8 @@ def validate_periods(periods: float | None) -> int | None:
periods = int(periods)
elif not lib.is_integer(periods):
raise TypeError(f"periods must be a number, got {periods}")
# error: Incompatible return value type (got "Optional[float]",
# expected "Optional[int]")
return periods # type: ignore[return-value]
periods = cast(int, periods)
return periods


def validate_inferred_freq(
Expand Down
4 changes: 3 additions & 1 deletion pandas/core/dtypes/cast.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ def _maybe_unbox_datetimelike(value: Scalar, dtype: DtypeObj) -> Scalar:
"""
if is_valid_na_for_dtype(value, dtype):
# GH#36541: can't fill array directly with pd.NaT
# > np.empty(10, dtype="datetime64[64]").fill(pd.NaT)
# > np.empty(10, dtype="datetime64[ns]").fill(pd.NaT)
# ValueError: cannot convert float NaN to integer
value = dtype.type("NaT", "ns")
elif isinstance(value, Timestamp):
Expand Down Expand Up @@ -800,6 +800,8 @@ def infer_dtype_from_scalar(val, pandas_dtype: bool = False) -> tuple[DtypeObj,
if val is NaT or val.tz is None: # type: ignore[comparison-overlap]
dtype = np.dtype("M8[ns]")
val = val.to_datetime64()
# TODO(2.0): this should be dtype = val.dtype
# to get the correct M8 resolution
else:
if pandas_dtype:
dtype = DatetimeTZDtype(unit="ns", tz=val.tz)
Expand Down