pandas-dev/pandas (original) (raw)

[ ] - Timestamp classmethods should return cls, not hard-coded Timestamp
[ ] - ... if and only if is_timestamp is updated to recognized subclasses.
[ ] - Timestamp._get_field does not need to be in the user-facing namespace
[x] - Timestamp._get_named_field does not need to be in the user-facing namespace
[ ] - Timestamp._get_start_end_field does not need to be in the user-facing namespace
[ ] - NaT / timedelta should behave like NaT / Timedelta #17955
[ ] - _assert_tzawareness_compat doesn't actually return the declared return type
[ ] - Is it the case that _localize_tso argument obj will always have obj.tzinfo == None? This is the case in all existing usages.

A few quick questions:

  1. __new__ returns Timestamp instead of cls. Is this intentional? Ditto Timestamp.replace returning Timestamp instead of self.__class__.
    • This might be solved by making create_timestamp_from_ts a classmethod. Maybe this function was written before cython supported cdef classmethods?
  2. Similarly, is_timestamp will miss subclasses of Timestamp. That is appreciably faster than isinstance(obj, Timestamp), so this may just be something we have to live with.
  3. Some Timestamp methods are cpdef but may not need to be user-facing, i.e. could just be cdef: _get_field, _get_named_field, _get_start_end_field
  4. Timestamp._get_named_field is only used once, for weekday_name. This calls
    val = self._maybe_convert_value_to_local()
    out = get_date_name_field(np.array([val], dtype=np.int64), field)
    return out[0]

which seems like huge overkill for return {0: 'Monday', ..., 6: 'Sunday'}[self.weekday()]. Was there a decision at some point that the latter is less performant?