Skip to content
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

fix: correct the error message for non-parquet file type and non-python api #15641

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

eitsupi
Copy link
Contributor

@eitsupi eitsupi commented Apr 14, 2024

The current error message is incorrect under several conditions and is worth correcting.

  1. write_parquet() etc. does not exist in Rust.
  2. A message will be displayed telling us to use write_parquet() for file formats other than Parquet.
In [1]: import polars as pl

In [2]: pl.DataFrame({"foo": [1]}).write_ipc("test.arrow")

In [3]: pl.scan_ipc("test.arrow").select(pl.col("foo") * 2).sink_ipc("test2.arrow")
---------------------------------------------------------------------------
InvalidOperationError                     Traceback (most recent call last)
Cell In[3], line 1
----> 1 pl.scan_ipc("test.arrow").select(pl.col("foo") * 2).sink_ipc("test2.arrow")

File ~/.local/lib/python3.10/site-packages/polars/_utils/unstable.py:59, in unstable.<locals>.decorate.<locals>.wrapper(*args, **kwargs)
     56 @wraps(function)
     57 def wrapper(*args: P.args, **kwargs: P.kwargs) -> T:
     58     issue_unstable_warning(f"`{function.__name__}` is considered unstable.")
---> 59     return function(*args, **kwargs)

File ~/.local/lib/python3.10/site-packages/polars/lazyframe/frame.py:2271, in LazyFrame.sink_ipc(self, path, compression, maintain_order, type_coercion, predicate_pushdown, projection_pushdown, simplify_expression, slice_pushdown, no_optimization)
   2221 """
   2222 Evaluate the query in streaming mode and write to an IPC file.
   2223 
   (...)
   2260 >>> lf.sink_ipc("out.arrow")  # doctest: +SKIP
   2261 """
   2262 lf = self._set_sink_optimizations(
   2263     type_coercion=type_coercion,
   2264     predicate_pushdown=predicate_pushdown,
   (...)
   2268     no_optimization=no_optimization,
   2269 )
-> 2271 return lf.sink_ipc(
   2272     path=path,
   2273     compression=compression,
   2274     maintain_order=maintain_order,
   2275 )

InvalidOperationError: sink_Ipc(IpcWriterOptions { compression: Some(ZSTD), maintain_order: true }) not yet supported in standard engine. Use 'collect().write_parquet()'

It could be changed to a longer error message including what to do in Rust and in Python, respectively, but I thought a shorter error message would be preferred here, given the other sections.

@github-actions github-actions bot added fix Bug fix python Related to Python Polars rust Related to Rust Polars labels Apr 14, 2024
Copy link

codspeed-hq bot commented Apr 14, 2024

CodSpeed Performance Report

Merging #15641 will not alter performance

Comparing eitsupi:error-message-update (45736a4) with main (37c6303)

Summary

✅ 21 untouched benchmarks

Copy link

codecov bot commented Apr 14, 2024

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 81.31%. Comparing base (7341aee) to head (45736a4).
Report is 2 commits behind head on main.

Files Patch % Lines
crates/polars-lazy/src/physical_plan/planner/lp.rs 0.00% 2 Missing ⚠️
crates/polars-plan/src/logical_plan/options.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #15641   +/-   ##
=======================================
  Coverage   81.30%   81.31%           
=======================================
  Files        1373     1373           
  Lines      176243   176244    +1     
  Branches     2544     2544           
=======================================
+ Hits       143293   143310   +17     
+ Misses      32466    32451   -15     
+ Partials      484      483    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

},
SinkType::File { file_type, .. } => {
polars_bail!(InvalidOperation:
"sink_{file_type:?} not yet supported in standard engine. Use 'collect().write_parquet()'"
"sink_{file_type:?} not yet supported in standard engine."
Copy link
Member

Choose a reason for hiding this comment

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

You can feature gate the error message. I want to keep the proper error message for python.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what your intent is, is 6168f1f okay?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix python Related to Python Polars rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants