-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Python] Repr of Timestamp scalars error if they are outside range of stdlib datetime.datetime #36323
Comments
Hey, I'd like to work on this issue. |
You can write 'take' into a comment body to assign yourself =) arrow/.github/workflows/comment_bot.yml Line 178 in e5de6a5
I will add this to the documentation: https://arrow.apache.org/docs/dev/developers/guide/step_by_step/finding_issues.html |
take |
@jorisvandenbossche should this be a blocker for 13.0.0 or can I move it to 14.0.0? |
This was referenced Jul 25, 2023
kou
pushed a commit
that referenced
this issue
Jul 31, 2023
…datetime range (#36942) ### Rationale for this change #36323 ### What changes are included in this PR? Changed the way repr is handled for TimestampScalar ### Are these changes tested? I have added a very basic test for this change to see whether it will error or not if outside the range. ### Are there any user-facing changes? The functionality of TimestampScalar's repr now uses the `strftime` function. * Closes: #36323 Lead-authored-by: Ashish Bailkeri <[email protected]> Co-authored-by: Ashish Bailkeri <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
R-JunmingChen
pushed a commit
to R-JunmingChen/arrow
that referenced
this issue
Aug 20, 2023
…tside datetime range (apache#36942) ### Rationale for this change apache#36323 ### What changes are included in this PR? Changed the way repr is handled for TimestampScalar ### Are these changes tested? I have added a very basic test for this change to see whether it will error or not if outside the range. ### Are there any user-facing changes? The functionality of TimestampScalar's repr now uses the `strftime` function. * Closes: apache#36323 Lead-authored-by: Ashish Bailkeri <[email protected]> Co-authored-by: Ashish Bailkeri <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
loicalleyne
pushed a commit
to loicalleyne/arrow
that referenced
this issue
Nov 13, 2023
…tside datetime range (apache#36942) ### Rationale for this change apache#36323 ### What changes are included in this PR? Changed the way repr is handled for TimestampScalar ### Are these changes tested? I have added a very basic test for this change to see whether it will error or not if outside the range. ### Are there any user-facing changes? The functionality of TimestampScalar's repr now uses the `strftime` function. * Closes: apache#36323 Lead-authored-by: Ashish Bailkeri <[email protected]> Co-authored-by: Ashish Bailkeri <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug, including details regarding any error messages, version, and platform.
Currently, the repr of the TimestampScalar (well, of scalars in general) essentially uses the repr of the Python object. For TimestampScalar, this thus fails if this conversion fails, such as for timestamps that are outside of the range of
datetime.datetime
([1-9999]):Our generic Scalar
__repr__
implementation:arrow/python/pyarrow/scalar.pxi
Lines 117 to 120 in 11b140a
I think for Timestamps we should just convert to a string repr ourselves (
strftime
, or our own pretty printer as is used for the Array repr)Component(s)
Python
The text was updated successfully, but these errors were encountered: