Skip to content

Respect WriteTar setting in Catapult writer#1490

Closed
niteshg97 wants to merge 4 commits into
fastmachinelearning:mainfrom
niteshg97:fix-catapult-write-tar
Closed

Respect WriteTar setting in Catapult writer#1490
niteshg97 wants to merge 4 commits into
fastmachinelearning:mainfrom
niteshg97:fix-catapult-write-tar

Conversation

@niteshg97

@niteshg97 niteshg97 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Description

Makes CatapultWriter respect the existing WriterConfig.WriteTar option.

Other writers only create a .tar.gz archive when WriteTar is enabled, but CatapultWriter always created one unconditionally, this makes catapult behavior consistent with the other writer backends.

This partially addresses #1438 by adding the missing WriteTar gate for Catapult.

Type of change

  • Bug fix (non breaking change that fixes an issue)

Tests

Added a focused regression test for CatapultWriter.write_tar().

Test Configuration:

focused writer unit test
No HLS toolchain required

pytest test/pytest/test_catapult_writer.py -q 

Checklist

  • I have read the contribution guidelines.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have installed and run pre-commit on the files I edited or added.
  • I have added tests that demonstrate my fix is effective or that my feature works as intended.

@niteshg97

Copy link
Copy Markdown
Contributor Author

pre-commit.ci autofix

@niteshg97

Copy link
Copy Markdown
Contributor Author

@jmitrevs Could you please review this PR? It makes the CatapultWriter respect the WriteTar setting, bringing it in line with the behavior of other writer backends. The fix includes a focused regression test that can be run with :

pytest test/pytest/test_catapult_writer.py -q

Would appreciate your input on the approach and would be great if this could also be added to the test suite. Thanks!

@JanFSchulte

Copy link
Copy Markdown
Contributor

Hi @niteshg97, thanks for your PR. However, this issue is already being addressed in #1442, so we will close here in favor of the existing PR.

@niteshg97

Copy link
Copy Markdown
Contributor Author

Hi @niteshg97, thanks for your PR. However, this issue is already being addressed in #1442, so we will close here in favor of the existing PR.

Thanks for letting me know, i missed that #1442 was already covering the WriteTar consistency work. Closing this in favor of the existing PR makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants