Skip to content

Conversation

@sycai
Copy link
Contributor

@sycai sycai commented Sep 26, 2024

No description provided.

@sycai sycai requested review from a team as code owners September 26, 2024 19:09
@sycai sycai requested a review from tswast September 26, 2024 19:09
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. labels Sep 26, 2024
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we make this [bigframes/pandas/io/parsers/readers.py](https://github.com/pandas-dev/pandas/blob/23c497bb2f7e05af1fda966e7fb04db942453559/pandas/io/parsers/readers.py#L720) to better match the pandas code layout?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or bigframes/pandas/io/api.py if we want to go one layer of indirection fewer

https://github.com/pandas-dev/pandas/blob/23c497bb2f7e05af1fda966e7fb04db942453559/pandas/__init__.py#L139

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I put all the functions under the api.py file since they are relatively simple

Copy link
Contributor

Choose a reason for hiding this comment

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

The docs of these methods are stayed at third_party/bigframes_vendored/pandas/io/parsers/readers.py. Should we keep them in the similar path?

@sycai sycai force-pushed the sycai-refactor-pandas branch from ed79ff2 to 18e52cc Compare September 26, 2024 20:12
@sycai sycai requested a review from chelsea-lin September 26, 2024 23:46
Copy link
Contributor

@chelsea-lin 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.

LGTM overall. Please check the two failing tests in the pre-submit.

@sycai sycai added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 27, 2024
@bigframes-bot bigframes-bot removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 27, 2024
@sycai sycai merged commit 2d18815 into main Sep 27, 2024
22 of 23 checks passed
@sycai sycai deleted the sycai-refactor-pandas branch September 27, 2024 15:51
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: l Pull request size is large.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants