DOC: Fixing EX01 - Added examples #53689
Conversation
There was a problem hiding this comment.
thanks @DeaMariaLeon !
Would be good to add a little test for these - could just add them to
pandas/pandas/tests/scalar/timestamp/test_timestamp.py
Lines 1123 to 1132 in cbad73a
There was a problem hiding this comment.
really sorry, noticed a few more things, so I'm "requesting changes" again
we should also add a whatsnew note to the 2.1.0.rst bug fixes section, noting that Timestamp.date was returning incorrect results for inputs outside those supported by the Python standard library's datetime module
pandas/_libs/tslibs/timestamps.pyx
Outdated
| "For now, please call the components you need (such as `.year` " | ||
| "and `.month`) and construct your string from there." |
There was a problem hiding this comment.
we're not constructing a string, so we can remove this part of the message
pandas/_libs/tslibs/timestamps.pyx
Outdated
|
|
||
| def dst(self): | ||
| """ | ||
| Return self.tzinfo.dst(self). |
There was a problem hiding this comment.
How about
Return the daylight saving time (DST) adjustment.
pandas/_libs/tslibs/timestamps.pyx
Outdated
| >>> ts = pd.Timestamp('2000-06-01 00:00:00', | ||
| ... tz='Europe/Brussels').dst() | ||
| >>> ts | ||
| datetime.timedelta(seconds=3600) |
There was a problem hiding this comment.
ts usually means "timestamp", so maybe let's do
>>> ts = pd.Timestamp('2000-06-01 00:00:00', tz='Europe/Brussels')
>>> ts.dst()
datetime.timedelta(seconds=3600)
pandas/_libs/tslibs/timestamps.pyx
Outdated
| _dt = datetime(self.year, self.month, self.day, | ||
| self.hour, self.minute, self.second, | ||
| self.microsecond, self.tzinfo, fold=self.fold) | ||
| return _dt.dst() |
There was a problem hiding this comment.
looks like we're not catching anything here, so can we just return return super().dst()?
maybe we can do the same for the others where we're just overriding the docstrings, like
EDIT: please disregard the second half of this message, only the first sentence (above) makes sense
pandas/_libs/tslibs/timestamps.pyx
Outdated
| try: | ||
| _dt = datetime(self.year, self.month, self.day, | ||
| self.hour, self.minute, self.second, | ||
| self.microsecond, self.tzinfo, fold=self.fold) |
There was a problem hiding this comment.
can we construct a date object directly, and then return that? like
_dt = date(self.year, self.month, self.day), where date is imported from datetime?
There was a problem hiding this comment.
Thanks for the corrections. :)
I did all of them locally except this one. I can't find how to import date from datetime without breaking it.
There was a problem hiding this comment.
might work to do import datetime as dt, then use dt.date
pandas/_libs/tslibs/timestamps.pyx
Outdated
| return self.weekday() + 1 | ||
| try: | ||
| _dt = datetime(self.year, self.month, self.day, | ||
| self.hour, self.minute, self.second, | ||
| self.microsecond, self.tzinfo, fold=self.fold) | ||
| except ValueError as err: | ||
| raise NotImplementedError( | ||
| "isoweekday not yet supported on Timestamps which " | ||
| "are outside the range of Python's standard library. " | ||
| ) from err | ||
| return _dt.weekday() + 1 |
There was a problem hiding this comment.
wait sorry, why did this one have to change? isn't self.weekday() + 1 correct?
There was a problem hiding this comment.
I thought that if we give it a negative date it should raise. So, in the stable version:
for ts = pd.Timestamp('-2000') the output is:
ts.isoweekday()=7
ts.weekday()=6
| func = "^isoweekday" | ||
| with pytest.raises(NotImplementedError, match=func + msg): | ||
| ts.isoweekday() |
There was a problem hiding this comment.
see my comment above, not sure this one needs to raise?
There was a problem hiding this comment.
Please see my question above.
doc/source/whatsnew/v2.1.0.rst
Outdated
| @@ -350,6 +350,7 @@ Datetimelike | |||
| ^^^^^^^^^^^^ | |||
| - :meth:`DatetimeIndex.map` with ``na_action="ignore"`` now works as expected. (:issue:`51644`) | |||
| - Bug in :func:`date_range` when ``freq`` was a :class:`DateOffset` with ``nanoseconds`` (:issue:`46877`) | |||
| - Bug in :meth:`Timestamp.date`, :meth:`Timestamp.isocalendar`, and :meth:`Timestamp.isoweekday` were returning incorrect results for inputs outside those supported by the Python standard library's datetime module (:issue:`53668`) | |||
There was a problem hiding this comment.
I think we can remove isoweekday
There was a problem hiding this comment.
Requested changes done.
* Added Timestamp examples * Added test and corrected msg * Correcting error on isoweekday * Added requested corrections * added import datetime * Removed isoweekday raise error, corrected time and timetz
Towards #37875
Partially fixing #53668