Skip to content

Make DTA _check_compatible_with less strict by default #30721

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 4 commits into from
Jan 6, 2020
Merged

Make DTA _check_compatible_with less strict by default #30721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fbe3800
Make DTA _check_compatible_with less strict by default
jbrockmendel Jan 6, 2020
8797ba9
Merge branch 'master' of https://github.com/pandas-dev/pandas into re…
jbrockmendel Jan 6, 2020
4527ab9
rename strict->setitem
jbrockmendel Jan 6, 2020
376545c
fixup
jbrockmendel Jan 6, 2020
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
9 changes: 6 additions & 3 deletions pandas/core/arrays/datetimelike.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def _unbox_scalar(self, value: Union[Period, Timestamp, Timedelta, NaTType]) ->
raise AbstractMethodError(self)

def _check_compatible_with(
self, other: Union[Period, Timestamp, Timedelta, NaTType]
self, other: Union[Period, Timestamp, Timedelta, NaTType], setitem: bool = False
) -> None:
"""
Verify that `self` and `other` are compatible.
Expand All @@ -123,6 +123,9 @@ def _check_compatible_with(
Parameters
----------
other
setitem : bool, default False
For __setitem__ we may have stricter compatiblity resrictions than
Copy link
Contributor

Choose a reason for hiding this comment

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

don’t we always need to check this for date times? why do u actually need a keyword

Copy link
Member Author

Choose a reason for hiding this comment

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

ATM we dont use _check_compatible_with in the DTA comparison ops, but after this we will be able to.

Copy link
Contributor

Choose a reason for hiding this comment

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

can you call this something else besides strict as this only applies to DTI, maybe check_tz (and ignore this elsewhere). i know strict is more generic, but also very confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

changed "strict"->"setitem"

for comparisons.

Raises
------
Expand Down Expand Up @@ -500,10 +503,10 @@ def __setitem__(
return

value = type(self)._from_sequence(value, dtype=self.dtype)
self._check_compatible_with(value)
self._check_compatible_with(value, setitem=True)
value = value.asi8
elif isinstance(value, self._scalar_type):
self._check_compatible_with(value)
self._check_compatible_with(value, setitem=True)
value = self._unbox_scalar(value)
elif is_valid_nat_for_dtype(value, self.dtype):
value = iNaT
Expand Down
9 changes: 6 additions & 3 deletions pandas/core/arrays/datetimes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,11 +554,14 @@ def _unbox_scalar(self, value):
def _scalar_from_string(self, value):
return Timestamp(value, tz=self.tz)

def _check_compatible_with(self, other):
def _check_compatible_with(self, other, setitem: bool = False):
if other is NaT:
return
if not timezones.tz_compare(self.tz, other.tz):
raise ValueError(f"Timezones don't match. '{self.tz} != {other.tz}'")
self._assert_tzawareness_compat(other)
if setitem:
# Stricter check for setitem vs comparison methods
if not timezones.tz_compare(self.tz, other.tz):
raise ValueError(f"Timezones don't match. '{self.tz} != {other.tz}'")

def _maybe_clear_freq(self):
self._freq = None
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/arrays/period.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ def _unbox_scalar(self, value: Union[Period, NaTType]) -> int:
def _scalar_from_string(self, value: str) -> Period:
return Period(value, freq=self.freq)

def _check_compatible_with(self, other):
def _check_compatible_with(self, other, setitem: bool = False):
if other is NaT:
return
if self.freqstr != other.freqstr:
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/arrays/timedeltas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ def _unbox_scalar(self, value):
def _scalar_from_string(self, value):
return Timedelta(value)

def _check_compatible_with(self, other):
def _check_compatible_with(self, other, setitem: bool = False):
# we don't have anything to validate.
pass

Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/arrays/test_datetimes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def test_tz_setter_raises(self):
def test_setitem_different_tz_raises(self):
data = np.array([1, 2, 3], dtype="M8[ns]")
arr = DatetimeArray(data, copy=False, dtype=DatetimeTZDtype(tz="US/Central"))
with pytest.raises(ValueError, match="None"):
with pytest.raises(TypeError, match="Cannot compare tz-naive and tz-aware"):
arr[0] = pd.Timestamp("2000")

with pytest.raises(ValueError, match="US/Central"):
Expand Down