diff --git a/.github/workflows/run-push-tests.yml b/.github/workflows/run-push-tests.yml deleted file mode 100644 index 5137067..0000000 --- a/.github/workflows/run-push-tests.yml +++ /dev/null @@ -1,32 +0,0 @@ -name: run-push-tests - -on: - push: - -jobs: - run-tests: - runs-on: ubuntu-latest - strategy: - matrix: - python-version: ["3.10", "3.13"] - - steps: - - name: Checkout repository - uses: actions/checkout@v4 - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5 - with: - python-version: ${{ matrix.python-version }} - - - name: install dependencies - run: pip install .[dev] - shell: bash - - - name: run Ruff linter - run: ruff check . - shell: bash - - - name: run tests - run: pytest - shell: bash diff --git a/.github/workflows/run-pr-tests.yml b/.github/workflows/run-tests.yml similarity index 60% rename from .github/workflows/run-pr-tests.yml rename to .github/workflows/run-tests.yml index 7853216..6a6481a 100644 --- a/.github/workflows/run-pr-tests.yml +++ b/.github/workflows/run-tests.yml @@ -1,12 +1,32 @@ -name: run-PR-tests +name: run-tests on: pull_request: push: - branches: main jobs: + lint: + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.13" + + - name: install dependencies + run: pip install .[dev] + shell: bash + + - name: run Ruff linter + run: ruff check --output-format=github . + shell: bash + run-tests: + needs: lint runs-on: ubuntu-latest strategy: matrix: @@ -24,10 +44,6 @@ jobs: - name: install dependencies run: pip install .[dev] shell: bash - - - name: run Ruff linter - run: ruff check . - shell: bash - name: run tests run: pytest diff --git a/.gitignore b/.gitignore index 26f8e80..2c5bbdc 100644 --- a/.gitignore +++ b/.gitignore @@ -111,6 +111,9 @@ ipython_config.py .pdm-python .pdm-build/ +# uv +uv.lock + # PEP 582; used by e.g. github.com/David-OConnor/pyflow and github.com/pdm-project/pdm __pypackages__/ diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ecc4517..8b786e5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,6 +1,9 @@ repos: - - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.11.2 + - repo: local hooks: - id: ruff - + name: ruff + entry: uv run --extra dev ruff check --force-exclude + language: system + types_or: [python, pyi] + require_serial: true \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml index ff89887..b4639e1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,13 +30,6 @@ dependencies = [ "sum-buddy", ] -[project.optional-dependencies] -dev = [ - "pytest", - "ruff", - "pre-commit" -] - keywords = [ "imageomics", "metadata", @@ -52,6 +45,13 @@ keywords = [ "sum-buddy", ] +[project.optional-dependencies] +dev = [ + "pytest", + "ruff==0.16.0", + "pre-commit" +] + [project.urls] Homepage = "https://github.com/Imageomics/cautious-robot" Repository = "https://github.com/Imageomics/cautious-robot.git" diff --git a/src/cautiousrobot/__init__.py b/src/cautiousrobot/__init__.py index 33d83c4..8e9b5cc 100644 --- a/src/cautiousrobot/__init__.py +++ b/src/cautiousrobot/__init__.py @@ -2,11 +2,4 @@ from cautiousrobot.download import download_images from cautiousrobot.utils import downsample_and_save_image -# Create instance of the class -buddy_check_instance = BuddyCheck() - -# Expose instance methods by executing them -buddy_check_instance.validate_download #noqa: B018 -buddy_check_instance.check_alignment #noqa: B018 - -__all__ = ["check_alignment", "download_images", "downsample_and_save_image", "validate_download"] +__all__ = ["BuddyCheck", "download_images", "downsample_and_save_image"] diff --git a/src/cautiousrobot/__main__.py b/src/cautiousrobot/__main__.py index 1548808..0c840c7 100644 --- a/src/cautiousrobot/__main__.py +++ b/src/cautiousrobot/__main__.py @@ -7,7 +7,6 @@ import argparse import hashlib -import sys import pandas as pd from sumbuddy import get_checksums @@ -15,6 +14,7 @@ from cautiousrobot.__about__ import __version__ from cautiousrobot.buddy_check import BuddyCheck from cautiousrobot.download import download_images +from cautiousrobot.exceptions import BuddyCheckError, ChecksumError from cautiousrobot.roll_call import RollCall from cautiousrobot.utils import process_csv @@ -77,9 +77,8 @@ def process_checksums(img_dir, metadata_path, args, source_df): print(f"There are {checksum_df.shape[0]} files in {img_dir}. Based on {args.input_file}, there should be {expected_num_imgs} images.") return checksum_df, expected_num_imgs - except Exception as e: # noqa: BLE001 - print(f"checksum calculation of downloaded images was unsuccessful due to {e}.") - print(f"you can get checksums for the images downloaded to {img_dir} by running sum-buddy directly.") + except (ChecksumError, OSError, ValueError, pd.errors.ParserError) as e: + print(e) return None, None @@ -102,10 +101,8 @@ def verify_downloads(args, source_df, checksum_df, filename_col, metadata_path, print(f"See {metadata_path}_missing.csv for missing image info and check logs.") else: print(f"Buddy check successful. All {expected_num_imgs} expected images accounted for.") - except Exception as e: # noqa: BLE001 - print(f"Verification of download failed due to {type(e).__name__}: {e}.") - print("'BuddyCheck.validate_download' can be run directly on DataFrames of the source and checksum CSVs after correcting for this error.") - + except (BuddyCheckError, OSError, ValueError, KeyError) as e: + print(e) def main(): args = parse_args() @@ -118,10 +115,8 @@ def main(): # Set up expected columns and process CSV expected_cols, subfolders = roll_call.setup_expected_columns(args) - try: - data_df = process_csv(csv_path, expected_cols) - except Exception as missing_cols: # noqa: BLE001 - sys.exit(f"{missing_cols} Please adjust inputs and try again.") + data_df = process_csv(csv_path, expected_cols) #Exception handling process_csv + # Validate data and handle missing filenames filename_col = expected_cols["filename_col"] diff --git a/src/cautiousrobot/download.py b/src/cautiousrobot/download.py index 4cfb954..d2a14c2 100644 --- a/src/cautiousrobot/download.py +++ b/src/cautiousrobot/download.py @@ -68,7 +68,7 @@ def get_content_type_from_url(url): content_type = response.headers.get('content-type', '') # Split off parameters like '; charset=utf-8' return content_type.split(';')[0].strip() if content_type else None - except Exception: # noqa: BLE001 + except requests.RequestException: return None @@ -273,7 +273,7 @@ def download_single_image(url, image_name, image_dir_path, log_data, log_errors, while redo and attempts_remaining > 0: try: response = requests.get(url, stream=True) - except Exception as e: # noqa: BLE001 + except requests.RequestException as error: redo = True attempts_remaining -= 1 if attempts_remaining <= 0: @@ -281,7 +281,7 @@ def download_single_image(url, image_name, image_dir_path, log_data, log_errors, index=i, image=image_name, file_path=url, - response_code=str(e)) + response_code=str(error)) update_log(log=log_errors, index=i, filepath=error_log_filepath) continue diff --git a/src/cautiousrobot/exceptions.py b/src/cautiousrobot/exceptions.py index aef3983..ddf4539 100644 --- a/src/cautiousrobot/exceptions.py +++ b/src/cautiousrobot/exceptions.py @@ -2,3 +2,23 @@ class EmptyDataFrameError(Exception): def __init__(self, df_name): message = f"Input DataFrame {df_name} is empty." super().__init__(message) + +class MissingColumnsError(Exception): + def __init__(self, csv_path, column_names, expected_columns): + message = f"The CSV at {csv_path} is missing column(s): {column_names}, defined as {expected_columns}." + super().__init__(message) + +class ChecksumError(Exception): + def __init__(self, error, img_dir): + self.error = error + self.img_dir = img_dir + message = f"Checksum calculation of downloaded images was unsuccessful due to {self.error}.\n You can get checksums for the images downloaded to {self.img_dir} by running sum-buddy directly." + super().__init__(message) + +class BuddyCheckError(Exception): + def __init__(self, error, reason): + self.error = error + self.reason = reason + message = f"Verification of download failed due to {self.error}: {self.reason}.\n'BuddyCheck.validate_download' can be run directly on DataFrames of the source and checksum CSVs after correcting for this error." + super().__init__(message) + \ No newline at end of file diff --git a/src/cautiousrobot/utils.py b/src/cautiousrobot/utils.py index 97561d4..0737bc0 100644 --- a/src/cautiousrobot/utils.py +++ b/src/cautiousrobot/utils.py @@ -4,7 +4,9 @@ import os import pandas as pd -from PIL import Image +from PIL import Image, UnidentifiedImageError + +from cautiousrobot.exceptions import MissingColumnsError def log_response(log_data, index, image, file_path, response_code): @@ -44,7 +46,7 @@ def process_csv(csv_path, expected_cols): if expected_cols[col] not in list(df.columns): missing_cols.append(col) if len(missing_cols) > 0: - raise Exception(f"The CSV at {csv_path} is missing column(s): {missing_cols}, defined as {[expected_cols[col] for col in missing_cols]}.") # noqa: TRY002 + raise MissingColumnsError(csv_path, missing_cols, [expected_cols[col] for col in missing_cols]) return df @@ -71,13 +73,12 @@ def downsample_and_save_image(image_dir_path, image_name, downsample_dir_path, d try: img = Image.open(f"{image_dir_path}/{image_name}") img.resize((downsample_size, downsample_size)).save(f"{downsample_dir_path}/{image_name}") - except Exception as e: # noqa: BLE001 - print(e) + except (OSError, UnidentifiedImageError) as error: log_errors = log_response( log_errors, index=image_index, image="downsized_" + image_name, file_path=file_path, - response_code=str(e) + response_code=str(error) ) update_log(log=log_errors, index=image_index, filepath=error_log_filepath) \ No newline at end of file diff --git a/tests/test_buddycheck.py b/tests/test_buddycheck.py index 171a8db..89b5eb0 100644 --- a/tests/test_buddycheck.py +++ b/tests/test_buddycheck.py @@ -14,22 +14,23 @@ def setUp(self): self.buddy_check_filename = BuddyCheck(buddy_id='filename') self.buddy_check_id_col = BuddyCheck(buddy_id = "filename", buddy_col = "sha256") - self.img_source_file = tempfile.NamedTemporaryFile(delete=False, mode='w') # noqa: SIM115 - self.checksum_source_file = tempfile.NamedTemporaryFile(delete=False, mode='w') # noqa: SIM115 + #self.img_source_file = tempfile.NamedTemporaryFile(delete=False, mode='w') + #self.checksum_source_file = tempfile.NamedTemporaryFile(delete=False, mode='w') - self.img_source_file.write("""filename,checksum + + with tempfile.NamedTemporaryFile(delete=False, mode='w', suffix=".csv") as f: + f.write("""filename,checksum image1.jpg,abc123 image2.jpg,def456 -image3.jpg,ghi789 -""") - self.img_source_file.close() +image3.jpg,ghi789""") + self.img_source_file = f - self.checksum_source_file.write("""filename,md5 + with tempfile.NamedTemporaryFile(delete=False, mode='w', suffix=".csv") as f: + f.write("""filename,md5 image1.jpg,abc123 image2.jpg,def456 -image3.jpg,ghi789 -""") - self.checksum_source_file.close() +image3.jpg,ghi789""") + self.checksum_source_file = f def tearDown(self): diff --git a/tests/test_download.py b/tests/test_download.py index 8dce3ee..0c9187b 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -2,6 +2,7 @@ from unittest.mock import patch import pandas as pd +import requests from cautiousrobot.download import download_images @@ -16,7 +17,7 @@ class TestDownload(unittest.TestCase): @patch('requests.get') def test_response_exception(self, get_mock): - get_mock.side_effect = Exception + get_mock.side_effect = requests.exceptions.ConnectionError("Connection failed") download_images(DUMMY_DATA, img_dir = IMG_DIR, log_filepath = LOG_FILEPATH, error_log_filepath = ERROR_LOG_FILEPATH) diff --git a/tests/test_download_images.py b/tests/test_download_images.py index 9e68c13..2155ad9 100644 --- a/tests/test_download_images.py +++ b/tests/test_download_images.py @@ -18,6 +18,7 @@ extract_extension_from_url, resolve_filename_with_extension, ) +from cautiousrobot.exceptions import MissingColumnsError TESTDATA_DIR = os.path.join(os.path.dirname(__file__), 'testdata') @@ -325,7 +326,8 @@ class TestMainFunction(unittest.TestCase): @patch('cautiousrobot.__main__.BuddyCheck') @patch('os.path.exists') @patch('builtins.input', return_value='y') - def test_main_successful_execution(self, mock_input, mock_exists, mock_BuddyCheck, mock_get_checksums, mock_download_images, mock_process_csv, mock_parse_args): + @patch('cautiousrobot.__main__.process_checksums') + def test_main_successful_execution(self, mock_process_checksums, mock_input, mock_exists, mock_BuddyCheck, mock_get_checksums, mock_download_images, mock_process_csv, mock_parse_args): mock_args = MagicMock() mock_args.input_file = 'test.csv' mock_args.img_name_col = 'filename_col' @@ -337,6 +339,7 @@ def test_main_successful_execution(self, mock_input, mock_exists, mock_BuddyChec mock_args.max_retries = 3 mock_args.checksum_algorithm = 'md5' mock_args.verifier_col = None + mock_process_checksums.return_value = (None, None) mock_parse_args.return_value = mock_args mock_exists.return_value = False @@ -374,12 +377,12 @@ def test_main_missing_columns_error(self, mock_process_csv, mock_parse_args): mock_args.subdir_col = None mock_parse_args.return_value = mock_args - mock_process_csv.side_effect = Exception("Missing required columns") + mock_process_csv.side_effect = MissingColumnsError("test.csv", ["filename"], ["filename_col"],) - with self.assertRaises(SystemExit) as cm: + with self.assertRaises(MissingColumnsError) as cm: main() - - self.assertEqual(cm.exception.code, "Missing required columns Please adjust inputs and try again.") + + self.assertEqual(str(cm.exception), "The CSV at test.csv is missing column(s): ['filename'], defined as ['filename_col'].") @patch('cautiousrobot.__main__.parse_args') @patch('cautiousrobot.__main__.process_csv') @@ -406,9 +409,12 @@ def test_main_non_unique_filenames(self, mock_process_csv, mock_parse_args): "filename_col is not a unique identifier for this dataset, please choose a column with unique values for filenames." ) - @patch('cautiousrobot.__main__.parse_args') + @patch('cautiousrobot.__main__.download_images') + @patch('cautiousrobot.__main__.get_checksums') + @patch('cautiousrobot.__main__.process_checksums') @patch('cautiousrobot.__main__.process_csv') - def test_main_missing_filenames_noninteractive(self, mock_process_csv, mock_parse_args): + @patch('cautiousrobot.__main__.parse_args') + def test_main_missing_filenames_noninteractive(self, mock_parse_args, mock_process_csv, mock_process_checksums, mock_get_checksums, mock_download_images): mock_args = MagicMock() mock_args.input_file = 'test.csv' mock_args.img_name_col = 'filename_col' @@ -428,6 +434,10 @@ def test_main_missing_filenames_noninteractive(self, mock_process_csv, mock_pars 'url_col': ['url1', 'url2', 'url3', 'url4'] }) + mock_process_csv.return_value = mock_data + mock_process_checksums.return_value = (None, None) + mock_get_checksums.return_value = None + mock_process_csv.return_value = mock_data # Should NOT exit now diff --git a/tests/test_downsample.py b/tests/test_downsample.py index dfa6d87..2689d21 100644 --- a/tests/test_downsample.py +++ b/tests/test_downsample.py @@ -96,7 +96,7 @@ def test_downsample_and_save_image_file_not_found(self, mock_update_log, mock_lo self.assertEqual(self.log_errors[0]['response_code'], "File not found") @patch("os.path.exists", return_value=False) - @patch("PIL.Image.open", side_effect=Exception("Unexpected error")) + @patch("PIL.Image.open", side_effect=OSError("Unexpected error")) @patch("cautiousrobot.utils.log_response") @patch("cautiousrobot.utils.update_log") def test_downsample_and_save_image_unexpected_error(self, mock_update_log, mock_log_response, mock_open, mock_exists): diff --git a/tests/test_roll_call.py b/tests/test_roll_call.py index e8cd109..be4b167 100644 --- a/tests/test_roll_call.py +++ b/tests/test_roll_call.py @@ -252,7 +252,7 @@ def test_columns_lowercase_conversion(self): args.url_col = "FileURL" args.subdir_col = None - expected_cols, subfolders = self.rollcall.setup_expected_columns(args) #noqa: RUF059 + expected_cols, _ = self.rollcall.setup_expected_columns(args) self.assertEqual(expected_cols["filename_col"], "filename") self.assertEqual(expected_cols["url_col"], "fileurl") @@ -326,7 +326,7 @@ def test_dictionary_structure(self): args.url_col = "url" args.subdir_col = "folder" - expected_cols, subfolders = self.rollcall.setup_expected_columns(args) #noqa: RUF059 + expected_cols, _ = self.rollcall.setup_expected_columns(args) # Check dictionary has correct keys self.assertIn("filename_col", expected_cols)