Large cleanup

Author: TomAugspurger

doc/source/whatsnew/v0.24.0.rst

@@ -678,7 +678,7 @@ Previous Behavior:
 .. code-block:: ipython
 
    In [2]: ser = pd.Series([pd.Timestamp('2000', tz='UTC'),
-                             pd.Timestamp('2000', tz='UTC')])
+      ...:                  pd.Timestamp('2000', tz='UTC')])
    In [3]: ser.unique()
    Out[3]: array([Timestamp('2000-01-01 00:00:00+0000', tz='UTC')], dtype=object)
 
@@ -1329,6 +1329,7 @@ Datetimelike
 - Bug in :func:`date_range` with frequency of ``Day`` or higher where dates sufficiently far in the future could wrap around to the past instead of raising ``OutOfBoundsDatetime`` (:issue:`14187`)
 - Bug in :class:`PeriodIndex` with attribute ``freq.n`` greater than 1 where adding a :class:`DateOffset` object would return incorrect results (:issue:`23215`)
 - Bug in :class:`Series` that interpreted string indices as lists of characters when setting datetimelike values (:issue:`23451`)
+- Bug in :class:`DataFrame` when creating a new column from an ndarray of :class:`Timestamp` objects with timezones converting creating an object-dtype column, rather than datetime with timezone (:issue:`23932`)
 - Bug in :class:`Timestamp` constructor which would drop the frequency of an input :class:`Timestamp` (:issue:`22311`)
 - Bug in :class:`DatetimeIndex` where calling ``np.array(dtindex, dtype=object)`` would incorrectly return an array of ``long`` objects (:issue:`23524`)
 - Bug in :class:`Index` where passing a timezone-aware :class:`DatetimeIndex` and `dtype=object` would incorrectly raise a ``ValueError`` (:issue:`23524`)

pandas/core/arrays/datetimelike.py

@@ -394,6 +394,10 @@ def asi8(self):
         # do not cache or you'll create a memory leak
         return self._data.view('i8')
 
+    @property
+    def _ndarray_values(self):
+        return self._data
+
     # ------------------------------------------------------------------
     # Formatting
 
@@ -514,9 +518,6 @@ def __setitem__(
             self._check_compatible_with(value)
             value = self._unbox_scalar(value)
         elif isna(value) or value == iNaT:
-            # TODO: Right now DatetimeTZBlock.fill_value is iNaT.
-            # There's some confuction about whether Block.fill_value should
-            # be the NA value or the storage value.
             value = iNaT
         else:
             msg = (
@@ -589,7 +590,6 @@ def astype(self, dtype, copy=True):
         #   3. DatetimeArray.astype handles datetime -> period
         from pandas import Categorical
         dtype = pandas_dtype(dtype)
-        # TODO: handle PeriodDtype, perhaps other EAs.
 
         if is_object_dtype(dtype):
             return self._box_values(self.asi8)
@@ -739,7 +739,7 @@ def repeat(self, repeats, *args, **kwargs):
         return type(self)(values, dtype=self.dtype)
 
     def map(self, mapper):
-        # TODO: remove this hack
+        # TODO(GH-23179): Add ExtensionArray.map
         # Need to figure out if we want ExtensionArray.map first.
         # If so, then we can refactor IndexOpsMixin._map_values to
         # a standalone function and call from here..
@@ -1339,16 +1339,12 @@ def _reduce(self, name, skipna=True, **kwargs):
     # Reductions
 
     def any(self, skipna=True):
-        if skipna:
-            values = self[~self.isnan]
-        else:
-            values = self
-
-        # TODO: Should any period be considered Falsey?
+        values = self._values_for_reduction(skipna=skipna)
         return len(values)
 
     def all(self, skipna=True):
-        return not self.all(skipna=skipna)
+        values = self._values_for_reduction(skipna=skipna)
+        return len(values)
 
     def _values_for_reduction(self, skipna=True):
         if skipna:

pandas/core/arrays/datetimes.py

@@ -242,11 +242,11 @@ def __init__(self, values, dtype=_NS_DTYPE, freq=None, copy=False):
                 dtype = DatetimeTZDtype(tz=dtype.tz)
             elif dtz and values.tz:
                 if not timezones.tz_compare(dtz, values.tz):
-                    # todo standard error message.
                     msg = (
-                        "Timezones do not match. {} != {}."
+                        "Timezone of the array and 'dtype' do not match. "
+                        "'{}' != '{}'"
                     )
-                    raise ValueError(msg.format(dtz, values.tz))
+                    raise TypeError(msg.format(dtz, values.tz))
             elif values.tz:
                 dtype = values.dtype
             # freq = validate_values_freq(values, freq)
@@ -273,7 +273,13 @@ def __init__(self, values, dtype=_NS_DTYPE, freq=None, copy=False):
             )
             raise ValueError(msg.format(values.dtype))
 
-        dtype = pandas_dtype(dtype)  # TODO: profile
+        # Performance note:
+        # for a length 10,000 ndarray[datetime64[ns]], pandas_dtype() takes
+        # ~15% of the runtime of __init__. It's only useful for converting
+        # string aliases like 'M8[ns]' or 'datetime64[ns, tz]'.
+        # We should consider requiring an actual dtype.
+
+        dtype = pandas_dtype(dtype)
         if (isinstance(dtype, np.dtype) and dtype != _NS_DTYPE
                 or not isinstance(dtype, (np.dtype, DatetimeTZDtype))):
             msg = (
@@ -436,7 +442,6 @@ def _scalar_from_string(self, value):
         return Timestamp(value, tz=self.tz)
 
     def _check_compatible_with(self, other):
-        # TODO: verify this.
         if not timezones.tz_compare(self.tz, other.tz):
             raise IncompatibleTimeZoneError(
                 "Timezone's don't match. '{} != {}'".format(self.tz, other.tz)
@@ -499,9 +504,9 @@ def _resolution(self):
     # Array-Like / EA-Interface Methods
 
     def __array__(self, dtype=None):
-        # TODO: Check PeriodArray.__array__ and push to parent
-        # This may need to wait for the deprecation of np.array
-        # on datetimetz data.
+        # TODO(datetime-tz __array__): push to parent
+        # If deprecating behavior for datetime-tz, we'll need to handle that
+        # specially.
         if is_object_dtype(dtype):
             return np.array(list(self), dtype=object)
         elif is_int64_dtype(dtype):
@@ -535,16 +540,8 @@ def __iter__(self):
     # ----------------------------------------------------------------
     # ExtensionArray Interface
 
-    @property
-    def _ndarray_values(self):
-        # TODO: Move to parent
-        return self._data
-
     @Appender(dtl.DatetimeLikeArrayMixin._validate_fill_value.__doc__)
     def _validate_fill_value(self, fill_value):
-        # TODO: Right now DatetimeTZBlock.fill_value is iNaT.
-        # There's some confuction about whether Block.fill_value should
-        # be the NA value or the storage value.
         if isna(fill_value) or fill_value == iNaT:
             fill_value = iNaT
         elif isinstance(fill_value, (datetime, np.datetime64)):
@@ -1094,7 +1091,6 @@ def astype(self, dtype, copy=True):
                 result = result._data
             return result
         elif is_datetime64tz_dtype(self.dtype) and self.dtype == dtype:
-            # TODO: add specific tests for each of these cases to arrays.
             if copy:
                 return self.copy()
             return self
@@ -1634,13 +1630,13 @@ def sequence_to_dt64ns(data, dtype=None, copy=False,
         data = data._data
 
     if isinstance(data, DatetimeArrayMixin):
-        # TODO: verify this changes. This was done in haste.
-        if inferred_freq and data.freq:
-            assert inferred_freq == data.freq
-
         if tz and data.tz:
             if not timezones.tz_compare(tz, data.tz):
-                raise TypeError("TODO")
+                msg = (
+                    "Timezone of the array and 'dtype' do not match. "
+                    "'{}' != '{}'"
+                )
+                raise TypeError(msg.format(tz, data.tz))
         tz = data.tz
         tz = validate_tz_from_dtype(dtype, tz)
 

pandas/core/arrays/period.py

@@ -258,11 +258,6 @@ def _check_compatible_with(self, other):
     def dtype(self):
         return self._dtype
 
-    @property
-    def _ndarray_values(self):
-        # Ordinals
-        return self._data
-
     @property
     def freq(self):
         """

pandas/core/arrays/timedeltas.py

@@ -285,8 +285,8 @@ def _formatter(self, boxed=False):
         return _get_format_timedelta64(self, box=True)
 
     def __array__(self, dtype=None):
-        # https://github.com/pandas-dev/pandas/pull/23593
-        # TODO: Check PeriodArray.__array__ and push to parent
+        # TODO(https://github.com/pandas-dev/pandas/pull/23593)
+        # Maybe push to parent once datetimetz __array__ is figured out.
         if is_object_dtype(dtype):
             return np.array(list(self), dtype=object)
         elif is_int64_dtype(dtype):
@@ -768,7 +768,6 @@ def astype(self, dtype, copy=True):
                 return values
             return result.astype('i8')
         elif is_timedelta64_ns_dtype(dtype):
-            # TODO: Figure out why this was needed.
             if copy:
                 return self.copy()
             return self

pandas/core/frame.py

@@ -4200,7 +4200,8 @@ def _maybe_casted_values(index, labels=None):
                 else:
                     values = values.take(labels)
 
-                    # TODO: Push this into maybe_upcast_putmask?
+                    # TODO(https://github.com/pandas-dev/pandas/issues/24206)
+                    # Push this into maybe_upcast_putmask?
                     # We can't pass EAs there right now. Looks a bit
                     # complicated.
                     # So we unbox the ndarray_values, op, re-box.

pandas/core/indexes/datetimelike.py

@@ -574,7 +574,6 @@ def _maybe_box_as_values(self, values, **attribs):
         return values
 
     def _deepcopy_if_needed(self, orig, copy=False):
-        # TODO: is this the right class?
         # Override Index._deepcopy_if_needed, since _data is not an ndarray.
         # what is orig here? ndarray or DatetimeArray, DatetimeIndex?
         if copy:
@@ -801,7 +800,6 @@ class DatelikeIndexMixin(object):
 
     @property
     def freq(self):
-        # TODO(DatetimeArray): remove
         # Can't simply use delegate_names since our base class is defining
         # freq
         return self._data.freq

pandas/core/indexes/datetimes.py

@@ -224,8 +224,6 @@ def _join_i8_wrapper(joinf, **kwargs):
     _tz = None
     _freq = None
     _comparables = ['name', 'freqstr', 'tz']
-    # TODO: decide whether freq is an attribute.
-    # Keeping it in attributes breaks things like Index.__getitem__
     _attributes = ['name', 'tz', 'freq']
 
     # dummy attribute so that datetime.__eq__(DatetimeArray) defers

pandas/core/indexes/timedeltas.py

@@ -160,7 +160,6 @@ def _join_i8_wrapper(joinf, **kwargs):
 
     _freq = None
 
-    # TODO: Deduplicate with DatetimeIndex by doing these as props on base
     _box_func = TimedeltaArray._box_func
     _bool_ops = TimedeltaArray._bool_ops
     _object_ops = TimedeltaArray._object_ops
@@ -220,7 +219,6 @@ def _simple_new(cls, values, name=None, freq=None, dtype=_TD_DTYPE):
         # `dtype` is passed by _shallow_copy in corner cases, should always
         #  be timedelta64[ns] if present
         if not isinstance(values, TimedeltaArray):
-            # TODO: make TimedeltaArray._simple_new idempotent?
             values = TimedeltaArray._simple_new(values, dtype=dtype,
                                                 freq=freq)
         assert isinstance(values, TimedeltaArray), type(values)

pandas/core/reshape/merge.py

@@ -1590,7 +1590,6 @@ def _right_outer_join(x, y, max_groups):
 
 def _factorize_keys(lk, rk, sort=True):
     if is_datetime64tz_dtype(lk) and is_datetime64tz_dtype(rk):
-        # TODO: verify if we get just arrays here, or maybe series / index
         lk = lk._data
         rk = rk._data
 

pandas/tests/arithmetic/test_datetime64.py

@@ -1471,7 +1471,6 @@ def check(get_ser, test_ser):
             # with 'operate' (from core/ops.py) for the ops that are not
             # defined
             op = getattr(get_ser, op_str, None)
-            # TODO: error message changed. Do we care?
             # Previously, _validate_for_numeric_binop in core/indexes/base.py
             # did this for us.
             with pytest.raises(TypeError,

pandas/tests/arrays/test_datetimes.py

@@ -13,6 +13,7 @@
 
 import pandas as pd
 from pandas.core.arrays import DatetimeArrayMixin as DatetimeArray
+from pandas.core.arrays.datetimes import sequence_to_dt64ns
 import pandas.util.testing as tm
 
 
@@ -21,7 +22,7 @@ def test_mismatched_timezone_raises(self):
         a = DatetimeArray(np.array(['2000-01-01T06:00:00'], dtype='M8[ns]'),
                           dtype=DatetimeTZDtype(tz='US/Central'))
         dtype = DatetimeTZDtype(tz='US/Eastern')
-        with pytest.raises(ValueError, match='Timezones'):
+        with pytest.raises(TypeError, match='do not match'):
             DatetimeArray(a, dtype=dtype)
 
     def test_non_array_raises(self):
@@ -105,3 +106,17 @@ def test_tz_setter_raises(self):
         arr = DatetimeArray._from_sequence(['2000'], tz='US/Central')
         with pytest.raises(AttributeError, match='tz_localize'):
             arr.tz = 'UTC'
+
+
+class TestSequenceToDT64NS(object):
+
+    def test_tz_dtype_mismatch_raises(self):
+        arr = DatetimeArray._from_sequence(['2000'], tz='US/Central')
+        with pytest.raises(TypeError, match='do not match'):
+            sequence_to_dt64ns(arr, dtype=DatetimeTZDtype(tz="UTC"))
+
+    def test_tz_dtype_matches(self):
+        arr = DatetimeArray._from_sequence(['2000'], tz='US/Central')
+        result, _, _ = sequence_to_dt64ns(
+            arr, dtype=DatetimeTZDtype(tz="US/Central"))
+        tm.assert_extension_array_equal(arr, result)

pandas/tests/extension/test_datetime.py

@@ -8,7 +8,6 @@
 from pandas.tests.extension import base
 
 
-# TODO: figure out a way to test non-TZ
 @pytest.fixture(params=["US/Central"])
 def dtype(request):
     return DatetimeTZDtype(unit="ns", tz=request.param)
@@ -117,8 +116,6 @@ class TestArithmeticOps(BaseDatetimeTests, base.BaseArithmeticOpsTests):
     implements = {'__sub__', '__rsub__'}
 
     def test_arith_series_with_scalar(self, data, all_arithmetic_operators):
-        # TODO: move this to the base class?
-        # It's duplicated between Period and Datetime now
         if all_arithmetic_operators in self.implements:
             s = pd.Series(data)
             self.check_opname(s, all_arithmetic_operators, s.iloc[0],

pandas/tests/test_base.py

@@ -1244,11 +1244,11 @@ def test_iter_box(self):
     # Series[M8[ns]] and Series[m8[ns]] to return a DateLikeArray.
     pytest.param(
         pd.DatetimeIndex(['2017', '2018']), np.ndarray, 'datetime64[ns]',
-        marks=[pytest.mark.xfail(reason="TODO", strict=True)]
+        marks=[pytest.mark.xfail(reason="datetime _values", strict=True)]
     ),
     pytest.param(
         pd.TimedeltaIndex([10**10]), np.ndarray, 'm8[ns]',
-        marks=[pytest.mark.xfail(reason="TODO", strict=True)]
+        marks=[pytest.mark.xfail(reason="timedelta _values", strict=True)]
     ),
 
 ])

pandas/tests/test_panel.py

@@ -470,11 +470,6 @@ def test_delitem_and_pop(self):
     def test_setitem(self):
         lp = self.panel.filter(['ItemA', 'ItemB']).to_frame()
 
-        # On master we go all the way down to
-        # MultiIndex.from_tuples(DatetimeIndex), which raise a
-        # ValueError: cannot include dtype 'M' in a buffer
-        # Now we (correctly) raise a TypeError.
-        # TODO: Add release note for this.
         with pytest.raises(TypeError):
             self.panel['ItemE'] = lp