Repository navigation
MAINT: Use utf8 strings in more of datetime - #17186
Conversation
eric-wieser
added
03 - Maintenance
component: numpy.datetime64
This fixes an omission in a previous patch that did not allow μs in datetime_as_string. This also will very slightly speed up most uses, since all but very unusual code will be passing in `str` not `bytes`.
24c9e1e to
68cfbd8
Compare
|
Looks like I screwed up the refcounting somewhere... |
seberg
left a comment
seberg
left a comment
There was a problem hiding this comment.
LGTM, I would prefer to change the Py_ssize_t since we are here. The other comments are just potential nitpicks.
| strobj = timezone_obj; | ||
| } | ||
|
|
||
| Py_SETREF(timezone_obj, strobj); |
There was a problem hiding this comment.
This SETREF feels a bit weird, since it is just strobj. But I guess you wanted to keep the unicode-convert code snippet is always identical?
There was a problem hiding this comment.
But I guess you wanted to keep the unicode-convert code snippet is always identical?
Yeah, I think that was my thought. I'd considered extracting it to a helper function a some point, likely bundling a deprecation warning because there's no reason to accept byte-strings here anyway.
Co-authored-by: Sebastian Berg <[email protected]>
|
Btw. I hesitated with merging, because I thought codecov has a point and some tests should just use byte strings. But I am willing to just push that some time myself, just didn't do it yet. |
|
Thanks Eric. |
3 participants
This fixes an omission in a previous patch (#17176) that did not allow μs in datetime_as_string.
This also will very slightly speed up most uses, since all but very unusual code will be passing in
strnotbytes.Motivated by #15317.