Skip to content

BUG: handle immutable arrays in tz_convert_from_utc (#35530) #35532

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

Merged
merged 1 commit into from
Aug 6, 2020

Conversation

lidavidm
Copy link
Contributor

@lidavidm lidavidm commented Aug 3, 2020

@lidavidm lidavidm force-pushed the gh35530 branch 3 times, most recently from 4b09568 to f60c88f Compare August 4, 2020 01:53
@WillAyd
Copy link
Member

WillAyd commented Aug 4, 2020

Seems reasonable; not sure if CI failure was related so restarted

@jbrockmendel

@WillAyd WillAyd added the Datetime Datetime data dtype label Aug 4, 2020
@jbrockmendel
Copy link
Member

small comments, otherwise lgtm

@@ -457,7 +457,7 @@ cdef int64_t[:] _tz_convert_from_utc(int64_t[:] vals, tzinfo tz):
str typ

if is_utc(tz):
converted = vals
return vals
Copy link
Member

Choose a reason for hiding this comment

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

doesnt need to be this PR, but might be convenient: i think this should probably return a copy, not the original array.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree this should be a copy, can you update @lidavidm otherwise lgtm. pls ping on green.

@jreback jreback added this to the 1.2 milestone Aug 5, 2020
@lidavidm
Copy link
Contributor Author

lidavidm commented Aug 5, 2020

Changed to be a copy (which also lets us remove the const specifiers pointed out), CI green!

@jbrockmendel
Copy link
Member

LGTM

@WillAyd WillAyd merged commit 69cd744 into pandas-dev:master Aug 6, 2020
@WillAyd
Copy link
Member

WillAyd commented Aug 6, 2020

Thanks @lidavidm

@lidavidm
Copy link
Contributor Author

lidavidm commented Aug 6, 2020

Thank you for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Datetime Datetime data dtype
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: "buffer source array is read-only" with tz_convert_from_utc/DatetimeArray.date
4 participants