Skip to content

refactor: move reader functions from __init__.py to a separate file under the pandas package #1023

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 11 commits into from
Sep 27, 2024

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.

4 participants