Skip to content

Conversation

Genesis929
Copy link
Collaborator

… is manually set.

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes b/310741105

@Genesis929 Genesis929 requested review from a team as code owners November 14, 2023 01:20
@Genesis929 Genesis929 requested a review from GarrettWu November 14, 2023 01:20
@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 Nov 14, 2023
) -> Tuple[pd.DataFrame, int, bigquery.QueryJob]:
"""Run query and download results as a pandas DataFrame. Return the total number of results as well."""
# TODO(swast): Allow for dry run and timeout.
enable_downsampling = bigframes.options.sampling.enable_downsampling
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you put code together for the same logic?

For example,
enable_downsampling = sampling_method is not None or bigframes.options.sampling.enable_downsampling

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

if random_state is None:
random_state = bigframes.options.sampling.random_state

sampling_method = sampling_method.lower()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you put sampling_method logic together?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@Genesis929 Genesis929 requested a review from jiaxunwu November 14, 2023 17:49
else bigframes.options.sampling.enable_downsampling
)

max_download_size = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: less code:

max_download_size = max_download_size or bigframes.options.sampling.max_download_size

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@Genesis929 Genesis929 requested a review from GarrettWu November 20, 2023 21:05
@Genesis929 Genesis929 merged commit ae03756 into main Nov 28, 2023
@Genesis929 Genesis929 deleted the b310741105-to-pandas-downsampling branch November 28, 2023 20:53
Genesis929 added a commit that referenced this pull request Dec 12, 2023
… is manually set. (#200)

* fix: make to_pandas override enable_downsampling when sampling_method is manually set.

* fix: make to_pandas override enable_downsampling when sampling_method is manually set.

* fix: make to_pandas override enable_downsampling when sampling_method is manually set.
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.

3 participants