-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
ENH: DatetimeTZDtype support non-nano #47120
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
Changes from all commits
78550cc
e045583
1916093
ad7adf9
b0cebad
0af55cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,8 @@ | |
import pytest | ||
import pytz | ||
|
||
from pandas._libs.tslibs.dtypes import NpyDatetimeUnit | ||
|
||
from pandas.core.dtypes.base import _registry as registry | ||
from pandas.core.dtypes.common import ( | ||
is_bool_dtype, | ||
|
@@ -263,10 +265,17 @@ def test_hash_vs_equality(self, dtype): | |
assert dtype2 != dtype4 | ||
assert hash(dtype2) != hash(dtype4) | ||
|
||
def test_construction(self): | ||
msg = "DatetimeTZDtype only supports ns units" | ||
def test_construction_non_nanosecond(self): | ||
res = DatetimeTZDtype("ms", "US/Eastern") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the Also would be good to have a full smoke test from one of the 1D constructors e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
would prefer to hold off; there's a check in DTA._simple_new that i haven't yet updated to handle non-nano dt64tz, so that'll get a dedicated (small) PR that makes a good fit for what you're suggesting |
||
assert res.unit == "ms" | ||
assert res._reso == NpyDatetimeUnit.NPY_FR_ms.value | ||
assert res.str == "|M8[ms]" | ||
assert str(res) == "datetime64[ms, US/Eastern]" | ||
|
||
def test_day_not_supported(self): | ||
msg = "DatetimeTZDtype only supports s, ms, us, ns units" | ||
with pytest.raises(ValueError, match=msg): | ||
DatetimeTZDtype("ms", "US/Eastern") | ||
DatetimeTZDtype("D", "US/Eastern") | ||
|
||
def test_subclass(self): | ||
a = DatetimeTZDtype.construct_from_string("datetime64[ns, US/Eastern]") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If base is analogous to "default" I think
M8[ns]
is a reasonable default.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed but not clear thats what 'base' means. would be good to have that documented somewhere.
The alternative (that ive put together for (Timestamp|Timedelta).(min|max|resolution)) is a property-like that assumes nanoseconds when accessed on the class and is instance-specific when accessed on an instance