Skip to content
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

BUG: non-nano strftime returns wrong results #50793

Merged
merged 2 commits into from
Jan 18, 2023

Conversation

MarcoGorelli
Copy link
Member

@MarcoGorelli MarcoGorelli marked this pull request as ready for review January 17, 2023 18:16
@MarcoGorelli
Copy link
Member Author

@jbrockmendel I don't think there's any prospect of getting this working before 2.0.0, or is there? OK with just raising an informative message? better than giving the wrong result imo 😄

@MarcoGorelli MarcoGorelli added Non-Nano datetime64/timedelta64 with non-nanosecond resolution Bug labels Jan 17, 2023
@jbrockmendel
Copy link
Member

better than giving the wrong result

Definitely.

It's a bit hacky, but what if we did something like

except ValueErrorOrWhatever:
    from pandas._libs.tslib import format_array_from_datetime
    values = np.array([self.value], dtype="i8")
    res = format_array_from_datetime(values, tz=self.tz, format=format, reso=self._creso)
    return res[0]

@MarcoGorelli
Copy link
Member Author

thanks - just tried that and got a long error ending with

  File "pandas/_libs/tslibs/timestamps.pyx", line 1417, in pandas._libs.tslibs.timestamps.Timestamp.strftime
    res, = format_array_from_datetime(values, tz=self.tz, format=format, reso=self._creso)
  File "pandas/_libs/tslib.pyx", line 207, in pandas._libs.tslib.format_array_from_datetime
    res = ts.strftime(format)
  File "pandas/_libs/tslibs/timestamps.pyx", line 1417, in pandas._libs.tslibs.timestamps.Timestamp.strftime
    res, = format_array_from_datetime(values, tz=self.tz, format=format, reso=self._creso)
  File "pandas/_libs/tslib.pyx", line 207, in pandas._libs.tslib.format_array_from_datetime
    res = ts.strftime(format)
  File "pandas/_libs/tslibs/timestamps.pyx", line 1417, in pandas._libs.tslibs.timestamps.Timestamp.strftime
    res, = format_array_from_datetime(values, tz=self.tz, format=format, reso=self._creso)
  File "pandas/_libs/tslib.pyx", line 207, in pandas._libs.tslib.format_array_from_datetime
    res = ts.strftime(format)
  File "pandas/_libs/tslibs/timestamps.pyx", line 1417, in pandas._libs.tslibs.timestamps.Timestamp.strftime
    res, = format_array_from_datetime(values, tz=self.tz, format=format, reso=self._creso)
  File "pandas/_libs/tslib.pyx", line 207, in pandas._libs.tslib.format_array_from_datetime
    res = ts.strftime(format)
  File "pandas/_libs/tslibs/timestamps.pyx", line 1413, in pandas._libs.tslibs.timestamps.Timestamp.strftime
    return self.to_pydatetime().strftime(format)
  File "pandas/_libs/tslibs/timestamps.pyx", line 1095, in pandas._libs.tslibs.timestamps._Timestamp.to_pydatetime
    cpdef datetime to_pydatetime(_Timestamp self, bint warn=True):
  File "pandas/_libs/tslibs/timestamps.pyx", line 1116, in pandas._libs.tslibs.timestamps._Timestamp.to_pydatetime
    return datetime(self.year, self.month, self.day,
RecursionError: maximum recursion depth exceeded while calling a Python object

@jbrockmendel
Copy link
Member

Makes sense. Next idea would be to do something like self.replace(year=9996).strftime(...).replace("9996", str(self.year))

@MarcoGorelli
Copy link
Member Author

maybe, but there'd be quite a lot of edge cases to take care of, and I'd be worried about rushing it

@jbrockmendel
Copy link
Member

Agreed.

@jbrockmendel
Copy link
Member

Maybe could handle the same subset of cases that array_strftime handles without recursing

try:
return self.to_pydatetime().strftime(format)
except ValueError as err:
raise ValueError(
Copy link
Member

Choose a reason for hiding this comment

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

NotImplementedError?

Copy link
Member Author

Choose a reason for hiding this comment

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

right, that'd be better, thanks!

@MarcoGorelli MarcoGorelli marked this pull request as draft January 18, 2023 14:33
@MarcoGorelli MarcoGorelli marked this pull request as ready for review January 18, 2023 15:58
@mroeschke mroeschke added this to the 2.0 milestone Jan 18, 2023
@mroeschke mroeschke merged commit d3ed92b into pandas-dev:main Jan 18, 2023
@jbrockmendel
Copy link
Member

looks like Timestamp.__repr__ calls strtime("%z"). can we support this case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Non-Nano datetime64/timedelta64 with non-nanosecond resolution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: non-nano strftime returns wrong results
3 participants