8000 feat: support datetime related casting in (Series|DataFrame|Index).astype by junyazhang · Pull Request #442 · googleapis/python-bigquery-dataframes · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@junyazhang
Copy link
Contributor

Fixes internal bug: b/328114618 🦕

@junyazhang junyazhang requested review from a team as code owners March 14, 2024 18:17
@junyazhang junyazhang requested a review from chelsea-lin March 14, 2024 18:17
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. labels Mar 14, 2024
@junyazhang junyazhang requested review from TrevorBergeron and m-strzelczyk and removed request for m-strzelczyk March 14, 2024 18:18
Copy link
Contributor
< 8000 img src="https://avatars.githubusercontent.com/u/124939984?s=48&v=4" alt="@chelsea-lin" size="24" height="24" width="24" data-view-component="true" class="avatar circle mr-2" /> chelsea-lin left a comment

Choose a reason for hiding this comment

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

Leave minor comment. Overall LGTM.

):
bf_result = scalars_df_index[column].astype(to_type).to_pandas()
pd_result = scalars_pandas_df_index[column].astype(to_type)
pd.testing.assert_series_equal(bf_result, pd_result, check_dtype=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add another assert to make sure the type of bf_result is expected, though it may be different as pd_result?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea!


if x.type() == ibis_dtypes.int64:
# The conversion unit is set to "us" (microseconds) for consistency
# with pandas converting timestamp[us][pyarrow] to int64[pyarrow].
Copy link
Contributor

Choose a reason for hiding this comment

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

Double check if the comment matches the code? It is converting int64[pyarrow] to timestamp[us][pyarrow], rather the opposite direction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Mar 15, 2024
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: l Pull request size is large. labels Mar 15, 2024
Copy link
Contributor
@TrevorBergeron TrevorBergeron left a comment

Choose a reason for hiding this comment

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

lgtm

@junyazhang junyazhang merged commit fde339b into main Mar 15, 2024
@junyazhang junyazhang deleted the main_lily_astype branch March 15, 2024 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

0