Skip to content

Conversation

TrevorBergeron
Copy link
Contributor

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 #<issue_number_goes_here> 🦕

@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 May 16, 2025
@TrevorBergeron TrevorBergeron marked this pull request as ready for review June 17, 2025 20:02
@TrevorBergeron TrevorBergeron requested review from a team as code owners June 17, 2025 20:02
@TrevorBergeron TrevorBergeron requested a review from GarrettWu June 17, 2025 20:02
@TrevorBergeron TrevorBergeron requested review from tswast and removed request for GarrettWu June 17, 2025 20:02
requests_transport_adapters: Sequence[
Tuple[str, requests.adapters.BaseAdapter]
] = (),
enable_polars_execution: bool = False,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be a compute option so it can be changed at runtime?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kind of just want to keep it as a constant within-session for now (will need to commit if making this a GA feature though). Turning polars execution on and off mid-session will make things like caching, multi-part execution really tricky

@TrevorBergeron TrevorBergeron requested a review from tswast June 18, 2025 19:22
tswast
tswast previously approved these changes Jun 25, 2025
return self._enable_polars_execution

@enable_polars_execution.setter
def enable_polars_execution(self, value: bool):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we add a check that the session has already started? If not (perhaps because we want to safely ignore this if the global session has already started), maybe add a comment for why.

    def enable_polars_execution(self, value: bool):
        if self._session_started and self._enable_polars_execution != value:
            raise ValueError(
                SESSION_STARTED_MESSAGE.format(attribute="enable_polars_execution")
            )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, should error out probably, as a session cannot change once started. error added in new revision

@TrevorBergeron TrevorBergeron requested a review from tswast June 26, 2025 16:50
@TrevorBergeron TrevorBergeron merged commit daf0c3b into main Jun 26, 2025
20 of 25 checks passed
@TrevorBergeron TrevorBergeron deleted the polars_semi branch June 26, 2025 17:57
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