Conversation
205b9c5 to
f43f938
Compare
f43f938 to
cd51e02
Compare
6ff46da to
9c87dbd
Compare
| # used only in testing right now, BigQueryCachingExecutor is the fully featured engine | ||
| # simplified, doest not do large >10 gb result queries, error handling, respect global config | ||
| # or record metrics | ||
| class DirectGbqExecutor(semi_executor.SemiExecutor): |
There was a problem hiding this comment.
I assume the purpose is to make sure caching and such don't cause any regressions / differences in behavior? Might be good to include that in the comment / docstring if so.
There was a problem hiding this comment.
yeah, mostly just to isolate the simplest, fastest version of bq execution, and avoid slow/complicated stuff only needed at >10gb scale or for stateful interactive flows. added comment in new revision
| # This will error out if polars is not installed | ||
| from bigframes.core.compile.polars import PolarsCompiler |
There was a problem hiding this comment.
Might be good to add a little helper to make sure folks know what package (and possibly versions) to install. See pandas helpers like this:
Or closer to home, the versions helpers in some of our client libraries:
There was a problem hiding this comment.
hmm, thats a good idea. I am going to expose polars execution in another PR soon, so I'll figure out the messaging experience in that one.
| from bigframes.testing import polars_session | ||
|
|
||
| session = polars_session.TestSession() | ||
| with bigframes.core.global_session._GlobalSessionContext(session): |
There was a problem hiding this comment.
Neat! Potentially something we want to consider exposing to users? I guess after someone asks for it so as not to pollute our public surface.
There was a problem hiding this comment.
Maybe if we get some requests, can productionize it. I'm worried its not robust to all the thread-local stuff however?
|
|
||
| pytest.importorskip("polars") | ||
|
|
||
| REFERENCE_ENGINE = polars_executor.PolarsExecutor() |
There was a problem hiding this comment.
I worry about this REFERENCE_ENGINE name. Might be worth a comment that the BigQuery engine is the source of truth, but we use this as the reference for faster testing?
There was a problem hiding this comment.
comment added
|
Doctest failure should be fixed by #1797 |
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:
Fixes #<issue_number_goes_here> 🦕