Skip to content

Implement compress #54

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

Open
wants to merge 12 commits into
base: directory
Choose a base branch
from
Open

Implement compress #54

wants to merge 12 commits into from

Conversation

samwaseda
Copy link
Member

It should be comparable to _job_compress in pyiron_base, with the difference that this one does not have the functionality to erase files, because I don’t have the feeling that it belongs here.

@samwaseda samwaseda requested review from liamhuber and Copilot August 3, 2025 07:39
Copy link

github-actions bot commented Aug 3, 2025

Binder 👈 Launch a binder notebook on branch pyiron/pyiron_snippets/compress

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements a compress method for the file handling functionality, which creates tar.gz archives of directories. The implementation is based on _job_compress from pyiron_base but excludes file deletion functionality.

  • Adds a compress method to create tar.gz archives with optional file exclusion
  • Includes comprehensive test coverage for the compression functionality
  • Prevents overwriting existing archives by checking for file existence

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
pyiron_snippets/files.py Implements the compress method with tarfile creation and exclude functionality
tests/unit/test_files.py Adds comprehensive unit tests for the compress method including exclusion scenarios

Comment on lines +100 to +104
def compress(self, exclude_files: list[str | Path] | None = None):
directory = self.path.resolve()
output_tar_path = directory.with_suffix(".tar.gz")
if output_tar_path.exists():
return
Copy link
Preview

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

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

The method silently returns without indication when the target file already exists. Consider raising an exception, returning a boolean status, or providing an overwrite parameter to make this behavior explicit to callers.

Suggested change
def compress(self, exclude_files: list[str | Path] | None = None):
directory = self.path.resolve()
output_tar_path = directory.with_suffix(".tar.gz")
if output_tar_path.exists():
return
def compress(self, exclude_files: list[str | Path] | None = None, overwrite: bool = False):
directory = self.path.resolve()
output_tar_path = directory.with_suffix(".tar.gz")
if output_tar_path.exists():
if not overwrite:
raise FileExistsError(f"Archive '{output_tar_path}' already exists. Set overwrite=True to overwrite.")
else:
output_tar_path.unlink()

Copilot uses AI. Check for mistakes.

@@ -95,3 +96,24 @@ def remove_files(self, *files: str):
path = self.get_path(file)
if path.is_file():
path.unlink()

def compress(self, exclude_files: list[str | Path] | None = None):
Copy link
Preview

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

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

The compress method lacks docstring documentation. It should document the purpose, parameters (especially the exclude_files behavior), return value, and the silent behavior when target file exists.

Suggested change
def compress(self, exclude_files: list[str | Path] | None = None):
def compress(self, exclude_files: list[str | Path] | None = None):
"""
Compress the directory into a `.tar.gz` archive, optionally excluding specified files.
Parameters
----------
exclude_files : list of str or pathlib.Path, optional
A list of file paths (relative to the directory or absolute) to exclude from the archive.
Each path is resolved relative to the directory if not absolute. Only files matching
the resolved paths are excluded; directories are not excluded recursively.
Returns
-------
None
Notes
-----
- If the target archive file (`<directory>.tar.gz`) already exists, the method does nothing and returns silently.
- Only files are added to the archive; directories themselves are not stored.
"""

Copilot uses AI. Check for mistakes.

Copy link

codecov bot commented Aug 3, 2025

Codecov Report

❌ Patch coverage is 93.75000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.11%. Comparing base (8cfcfa8) to head (4ff35e8).

Files with missing lines Patch % Lines
pyiron_snippets/files.py 93.75% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           directory      #54      +/-   ##
=============================================
- Coverage      95.22%   95.11%   -0.12%     
=============================================
  Files             13       13              
  Lines            503      532      +29     
=============================================
+ Hits             479      506      +27     
- Misses            24       26       +2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant