feat: Add support for `IO[bytes]` and `bytes` in `scan_{...}` functions #18532

coastalwhite · 2024-09-03T15:37:05Z

This PR adds support for the bytes and IO[bytes] interface to all scan_{parquet, csv, ...} functions.

Since the same code was being touched. This PR also fixes #18581.

coastalwhite · 2024-09-05T14:55:01Z

crates/polars-io/src/csv/read/parser.rs

 separator: u8,
 quote_char: Option<u8>,
 comment_prefix: Option<&CommentPrefix>,
 eol_char: u8,
 has_header: bool,
 ) -> PolarsResult<usize> {
 let file = if is_cloud_url(path) || config::force_async() {
- #[cfg(feature = "cloud")]
- {
+ feature_gated!("cloud", {


drive-by: this is the preferred way

Yes, much nicer.

coastalwhite · 2024-09-05T14:59:08Z

crates/polars-lazy/src/scan/ndjson.rs

 let file_options = FileScanOptions {
 slice: self.n_rows.map(|x| (0, x)),
 with_columns: None,
 cache: false,
 row_index: self.row_index,
 rechunk: self.rechunk,
 file_counter: 0,
- hive_options: Default::default(),
+ hive_options: HiveOptions {
+ enabled: Some(false),


HiveOptions::default sets enabled to Some(true) this is problematic. So I changed it here.

coastalwhite · 2024-09-05T15:05:03Z

crates/polars-mem-engine/src/executors/scan/csv.rs

+
+ let memslice = match source {
+ ScanSourceRef::File(path) => {
+ let file = if run_async {


drive-by: feature_gated is preferred

coastalwhite · 2024-09-05T15:06:17Z

crates/polars-mem-engine/src/executors/scan/ipc.rs

-
- #[cfg(feature = "cloud")]
- {
+ feature_gated!("cloud", {


drive-by: feature_gated is preferred

coastalwhite · 2024-09-05T15:11:11Z

crates/polars-mem-engine/src/executors/scan/ipc.rs

@@ -11,7 +10,7 @@ use rayon::prelude::*;
 use super::*;

 pub struct IpcExec {


We remove the options here, basically because the memory map options do not do anything anymore

coastalwhite · 2024-09-06T09:31:54Z

crates/polars-mem-engine/src/utils.rs

+/// # Notes
+///
+/// - Scan sources with in-memory buffers are ignored.
+pub(crate) fn agg_source_paths<'a>(


drive-by: remove the allocation here

coastalwhite · 2024-09-06T16:32:23Z

Putting this back into draft for a second. I want to try to remove the specialized read_{_} impls as well with this PR now so I made it possible to pass opened files. This did create some further problems, which I still need to fix.

coastalwhite · 2024-09-08T13:54:20Z

Putting this back into draft for a second. I want to try to remove the specialized read_{_} impls as well with this PR now so I made it possible to pass opened files. This did create some further problems, which I still need to fix.

In the end I chose not to do this, because it would require significant mappings.

coastalwhite · 2024-09-08T14:06:58Z

py-polars/tests/unit/io/test_ipc.py

@@ -339,29 +340,3 @@ def test_ipc_decimal_15920(
 path = f"{tmp_path}/data"
 df.write_ipc(path)
 assert_frame_equal(pl.read_ipc(path), df)
-


I removed this test as it does not really make much sense. The mmaps are unmapped when they are finished, I feel like a bug was triggering this behavior before. For the streaming engine it does make a lot of sense to have a test though and the checks for this are still in place.

coastalwhite · 2024-09-08T14:17:23Z

This solves one of the subgoals of #13040.

codecov · 2024-09-08T14:22:16Z

Codecov Report

Attention: Patch coverage is 81.60813% with 199 lines in your changes missing coverage. Please review.

Project coverage is 79.89%. Comparing base (6076421) to head (1e2fa0d).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/polars-python/src/file.rs	79.31%	24 Missing ⚠️
crates/polars-plan/src/plans/ir/scan_sources.rs	83.91%	23 Missing ⚠️
crates/polars-lazy/src/scan/csv.rs	65.00%	21 Missing ⚠️
crates/polars-plan/src/plans/functions/count.rs	75.00%	21 Missing ⚠️
...ates/polars-plan/src/plans/optimizer/count_star.rs	15.00%	17 Missing ⚠️
crates/polars-python/src/conversion/mod.rs	73.68%	10 Missing ⚠️
crates/polars-lazy/src/scan/file_list_reader.rs	0.00%	8 Missing ⚠️
crates/polars-plan/src/plans/functions/mod.rs	69.23%	8 Missing ⚠️
crates/polars-lazy/src/scan/ndjson.rs	66.66%	6 Missing ⚠️
crates/polars-plan/src/plans/conversion/scans.rs	93.25%	6 Missing ⚠️
... and 22 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #18532      +/-   ##
==========================================
- Coverage   79.93%   79.89%   -0.04%     
==========================================
  Files        1505     1506       +1     
  Lines      202628   203040     +412     
  Branches     2873     2889      +16     
==========================================
+ Hits       161976   162226     +250     
- Misses      40104    40264     +160     
- Partials      548      550       +2

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

coastalwhite · 2024-09-08T14:26:56Z

In the end, a lot more things had to be touched than I thought originally. @ritchie46 sorry for the large review 😓

crates/polars-io/src/ipc/ipc_file.rs

github-actions bot added enhancement New feature or an improvement of an existing feature python Related to Python Polars rust Related to Rust Polars labels Sep 3, 2024

coastalwhite force-pushed the feat-bytesio-scan branch from 06161e2 to 1328ea6 Compare September 5, 2024 14:54

coastalwhite commented Sep 5, 2024

View reviewed changes

coastalwhite marked this pull request as ready for review September 5, 2024 15:19

coastalwhite requested review from ritchie46, c-peters, alexander-beedie, MarcoGorelli, reswqa and orlp as code owners September 5, 2024 15:19

coastalwhite commented Sep 6, 2024

View reviewed changes

coastalwhite marked this pull request as draft September 6, 2024 16:30

coastalwhite added 15 commits September 8, 2024 13:55

feat: Allow BytesIO for Parquet scan

5c29bd7

refactor: Add ScanSource to CountStar

e14b78c

refactor: Add ScanSource to other scan functions

3e72f51

working for all scans

5fb9ffa

almost completely working :)

87aef71

no more failing tests

278e1a7

minor fixes

88e7651

fix cfg

6026101

fix several issues

ef8d034

fix: pola-rs#18581

0493270

fix StringIO loading for scan_csv, scan_ndjson

b97b529

fix async and source lists

49bcc85

small fixes

3070d7e

fix a million mypy errors

8b4b523

mypy

17b90b9

coastalwhite added 2 commits September 8, 2024 13:55

almost working further file support

daae7f1

add bytes and file descriptors

0b49b13

coastalwhite force-pushed the feat-bytesio-scan branch from fbdbe4c to 0b49b13 Compare September 8, 2024 11:55

mypy

be81e80

coastalwhite changed the title ~~feat: Add support for BytesIO in scan_{...} functions~~ feat: Add support for IO[bytes] and bytes in scan_{...} functions Sep 8, 2024

coastalwhite added 3 commits September 8, 2024 14:01

add bytes to ndjson

29fe063

clean up the code a bit

5daebe4

fix mypy

393e589

coastalwhite marked this pull request as ready for review September 8, 2024 12:43

coastalwhite added 2 commits September 8, 2024 15:46

fix mmap

1ddc3a5

clippy

124b548

coastalwhite added 2 commits September 8, 2024 16:04

remove broken test

b288811

ruff

a3ffac6

coastalwhite commented Sep 8, 2024

View reviewed changes

ritchie46 approved these changes Sep 9, 2024

View reviewed changes

crates/polars-io/src/ipc/ipc_file.rs Outdated Show resolved Hide resolved

coastalwhite added 2 commits September 9, 2024 09:17

remove stale memory_map option

e35eabe

fix test

1e2fa0d

ritchie46 added the highlight Highlight this PR in the changelog label Sep 9, 2024

ritchie46 merged commit ac2456c into pola-rs:main Sep 9, 2024
27 checks passed

coastalwhite deleted the feat-bytesio-scan branch September 9, 2024 08:28

cmdlineluser mentioned this pull request Sep 9, 2024

scan_parquet from io.BytesIO() #10413

Open

squnit mentioned this pull request Sep 11, 2024

refactor(rust): Allow polars to pass cargo check on windows #18672

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add support for `IO[bytes]` and `bytes` in `scan_{...}` functions #18532

feat: Add support for `IO[bytes]` and `bytes` in `scan_{...}` functions #18532

coastalwhite commented Sep 3, 2024 •

edited

Loading

coastalwhite Sep 5, 2024

ritchie46 Sep 9, 2024

coastalwhite Sep 5, 2024

coastalwhite Sep 5, 2024

coastalwhite Sep 5, 2024

coastalwhite Sep 5, 2024

coastalwhite Sep 6, 2024

coastalwhite commented Sep 6, 2024

coastalwhite commented Sep 8, 2024

coastalwhite Sep 8, 2024

coastalwhite commented Sep 8, 2024

codecov bot commented Sep 8, 2024 •

edited

Loading

coastalwhite commented Sep 8, 2024

		@@ -11,7 +10,7 @@ use rayon::prelude::*;
		use super::*;

		pub struct IpcExec {

feat: Add support for IO[bytes] and bytes in scan_{...} functions #18532

feat: Add support for IO[bytes] and bytes in scan_{...} functions #18532

Conversation

coastalwhite commented Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coastalwhite commented Sep 6, 2024

coastalwhite commented Sep 8, 2024

Choose a reason for hiding this comment

coastalwhite commented Sep 8, 2024

codecov bot commented Sep 8, 2024 • edited Loading

Codecov Report

coastalwhite commented Sep 8, 2024

feat: Add support for `IO[bytes]` and `bytes` in `scan_{...}` functions #18532

feat: Add support for `IO[bytes]` and `bytes` in `scan_{...}` functions #18532

coastalwhite commented Sep 3, 2024 •

edited

Loading

codecov bot commented Sep 8, 2024 •

edited

Loading