Skip to content

bz2: return streaming output from BZ2Compressor.compress() - #8330

Merged
youknowone merged 2 commits into
RustPython:mainfrom
doma17:fix-bz2-streaming-output
Jul 20, 2026
Merged

bz2: return streaming output from BZ2Compressor.compress()#8330
youknowone merged 2 commits into
RustPython:mainfrom
doma17:fix-bz2-streaming-output

Conversation

@doma17

@doma17 doma17 commented Jul 20, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

AI assistance: Codex gpt-5.6-sol

I reviewed and manually verified the change.

Summary

Return output produced by BZ2Compressor.compress() instead of waiting until
flush().

This restores incremental compression output and lets testDecompressorChunksMaxsize run without its RustPython expected-failure marker.

Testing

  • cargo run --release -- -m test test_bz2
  • cargo run -- extra_tests/snippets/stdlib_bz2.py
  • prek run --all-files
  • Compared incremental output and round-trip decompression with CPython

Summary by CodeRabbit

  • Bug Fixes

    • Fixed BZ2 compression so compress() returns available compressed data immediately instead of an empty result.
    • Ensured streaming compression output can be combined with flush() output and successfully decompressed.

  • Tests

    • Added coverage for partial BZ2 compression output and full payload reconstruction.

@doma17
bz2: return streaming output from BZ2Compressor.compress()
b2fa3ad 
  Return output produced during compression instead of waiting for flush().

  Constraint: Preserve encoder state without flushing the BZ2 stream
  Confidence: high
  Scope-risk: narrow
  Directive: Do not flush merely to retrieve partial output
  Tested: test_bz2; stdlib_bz2.py; prek run --all-files
  Assisted-by: Codex:gpt-5.6-sol
@coderabbitai

coderabbitai Bot commented Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

BZ2Compressor.compress() now returns pending compressed bytes instead of an empty result. A new test verifies partial output and decompression after combining it with flush() output.

Changes

BZ2 streaming output

Layer / File(s) Summary
Return pending compressor output
crates/stdlib/src/bz2.rs		 BZ2Compressor.compress() flushes pending encoder output and returns the buffered bytes using mem::take.
Validate streaming behavior
extra_tests/snippets/stdlib_bz2.py		 The test asserts that compression produces partial output and that partial plus final output decompresses to the original payload.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the main change: BZ2Compressor.compress() now returns streaming output.
Linked Issues check ✅ Passed The code and new test align with the issue by making compress() return non-empty intermediate output while preserving correct decompression.
Out of Scope Changes check ✅ Passed The changes stay focused on bz2 streaming output and a matching test snippet, with no unrelated code changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] lib: cpython/Lib/bz2.py
[x] test: cpython/Lib/test/test_bz2.py (TODO: 1)

dependencies:

  • bz2

dependent tests: (101 tests)

  • bz2: test_bz2 test_codecs test_fileinput test_tarfile
    • fileinput: test_genericalias
    • shutil: test_argparse test_compileall test_ctypes test_embed test_filecmp test_glob test_httpservers test_importlib test_inspect test_largefile test_launcher test_logging test_modulefinder test_os test_peg_generator test_pkgutil test_py_compile test_reprlib test_sax test_shutil test_site test_string_literals test_subprocess test_support test_sysconfig test_tempfile test_traceback test_unicode_file test_venv test_zoneinfo
      • ctypes.util: test_ctypes
      • ensurepip: test_ensurepip
      • http.server: test_robotparser test_urllib2_localnet test_xmlrpc
      • multiprocessing.util: test_asyncio test_concurrent_futures
      • pathlib: test_ast test_dbm_sqlite3 test_importlib test_json test_pathlib test_pyrepl test_runpy test_tomllib test_tools test_unparse test_winapi test_zipapp test_zipfile test_zstd
      • tempfile: test_asyncio test_bytes test_cmd_line test_compile test_concurrent_futures test_contextlib test_cprofile test_csv test_dis test_doctest test_faulthandler test_generated_cases test_hashlib test_importlib test_linecache test_mailbox test_ntpath test_pickle test_pkg test_posix test_pstats test_pydoc test_pyrepl test_regrtest test_selectors test_socket test_sys test_sys_settrace test_tabnanny test_termios test_threadedtempfile test_tokenize test_turtle test_urllib test_urllib2 test_urllib_response test_winconsoleio test_zipfile test_zipfile64
      • webbrowser: test_webbrowser
      • zipapp: test_pdb
      • zipfile: test_zipfile test_zipimport test_zipimport_support
    • zipfile:
      • importlib.metadata: test_importlib

[x] lib: cpython/Lib/csv.py
[x] test: cpython/Lib/test/test_csv.py (TODO: 20)

dependencies:

  • csv

dependent tests: (4 tests)

  • csv: test_csv test_genericalias
    • importlib.metadata: test_importlib test_zoneinfo

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

coderabbitai[bot]
coderabbitai Bot reviewed Jul 20, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/stdlib/src/bz2.rs`:
- Around line 169-173: Update the encoder handling in the bz2 compression flow
around encoder.as_mut(), replacing unwrap() with ok_or_else that returns a
Python ValueError when the encoder is unavailable. Propagate write_all and the
empty write errors through map_err as Python OS errors, remove the assert_eq!,
and use ? while ignoring the empty write’s byte count.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 1dce455e-0c8b-4ef7-89f7-7fad4f4503ff

📥 Commits

Reviewing files that changed from the base of the PR and between eb56114 and b2fa3ad.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_bz2.py is excluded by !Lib/**
📒 Files selected for processing (2)
  • crates/stdlib/src/bz2.rs
  • extra_tests/snippets/stdlib_bz2.py

Comment thread crates/stdlib/src/bz2.rs
ShaharNaveh
ShaharNaveh reviewed Jul 20, 2026
View reviewed changes
Comment thread extra_tests/snippets/stdlib_bz2.py Outdated

@ShaharNaveh ShaharNaveh Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

isn't cpython already tests that?

@doma17 doma17 Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That was my mistake. TY

ShaharNaveh
ShaharNaveh approved these changes Jul 20, 2026
View reviewed changes

@ShaharNaveh ShaharNaveh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

tysm!

@youknowone
youknowone merged commit 691ee06 into RustPython:main Jul 20, 2026
27 checks passed
youknowone pushed a commit that referenced this pull request Sep 16, 2026
@doma17
bz2: return streaming output from BZ2Compressor.compress() (#8330)
d50071e 
* bz2: return streaming output from BZ2Compressor.compress()

  Return output produced during compression instead of waiting for flush().

  Constraint: Preserve encoder state without flushing the BZ2 stream
  Confidence: high
  Scope-risk: narrow
  Directive: Do not flush merely to retrieve partial output
  Tested: test_bz2; stdlib_bz2.py; prek run --all-files
  Assisted-by: Codex:gpt-5.6-sol

* bz2: remove redundant streaming-output snippet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments
@ShaharNaveh ShaharNaveh ShaharNaveh approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bz2: return streaming output from BZ2Compressor.compress()

3 participants

@doma17 @ShaharNaveh @youknowone