Skip to content
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

Add pad #2197

Merged
merged 2 commits into from
Dec 13, 2024
Merged

Add pad #2197

merged 2 commits into from
Dec 13, 2024

Conversation

ternaus
Copy link
Collaborator

@ternaus ternaus commented Dec 13, 2024

Summary by Sourcery

Add a new Pad3D class for padding 3D volumes, refactor existing padding logic into a BasePad3D class, and update tests and documentation accordingly. Deprecate old padding parameters with warnings for backward compatibility.

New Features:

  • Introduce Pad3D class for padding 3D volumes with specified padding values and fill options.

Enhancements:

  • Refactor PadIfNeeded3D to inherit from a new BasePad3D class, improving code reuse and structure.
  • Deprecate old padding-related parameters in favor of new ones with warnings for backward compatibility.

Documentation:

  • Add documentation entry for the new Pad3D transform in the README.

Tests:

  • Add tests for the new Pad3D class, covering various padding configurations and data types.
  • Update existing tests to accommodate changes in padding parameter names and deprecations.

Chores:

  • Update pre-commit configuration to use ruff version 0.8.3.

Vladimir Iglovikov added 2 commits December 13, 2024 15:36
Copy link
Contributor

sourcery-ai bot commented Dec 13, 2024

Reviewer's Guide by Sourcery

This PR introduces a new Pad3D transform class and refactors padding-related code by extracting common functionality into a base class BasePad3D. The changes improve code organization and add new padding capabilities for 3D volumes.

Class diagram for the new Pad3D and BasePad3D classes

classDiagram
    class Transform3D {
        <<abstract>>
    }

    class BasePad3D {
        fill: ColorType
        fill_mask: ColorType
        apply_to_images(images: np.ndarray, padding: tuple[int, int, int, int, int, int], **params: Any) np.ndarray
        apply_to_masks(masks: np.ndarray, padding: tuple[int, int, int, int, int, int], **params: Any) np.ndarray
    }

    class Pad3D {
        padding: int | tuple[int, int, int] | tuple[int, int, int, int, int, int]
        fill: ColorType
        fill_mask: ColorType
        get_params_dependent_on_data(params: dict[str, Any], data: dict[str, Any]) dict[str, Any]
        get_transform_init_args_names() tuple[str, ...]
    }

    class PadIfNeeded3D {
        min_zyx: tuple[int, int, int] | None
        pad_divisor_zyx: tuple[int, int, int] | None
        position: Literal["center", "random"]
    }

    Transform3D <|-- BasePad3D
    BasePad3D <|-- Pad3D
    BasePad3D <|-- PadIfNeeded3D
    note for BasePad3D "Base class for 3D padding transforms"
Loading

Class diagram for InitSchema changes

classDiagram
    class InitSchema {
        fill: ColorType
        fill_mask: ColorType
        pad_mode: BorderModeType | None
        pad_cval: ColorType | None
        pad_cval_mask: ColorType | None
        validate_dimensions() Self
        validate_coordinates() Self
        validate_fill_types() Self
        validate_value() Self
    }

    class BasePad3DInitSchema {
        fill: ColorType
        fill_mask: ColorType
    }

    InitSchema <|-- BasePad3DInitSchema
    note for InitSchema "Handles validation and deprecation warnings"
Loading

File-Level Changes

Change Details Files
Added new Pad3D transform for explicit 3D volume padding
  • Supports single integer padding (same for all sides)
  • Supports 3-tuple padding (symmetric per dimension)
  • Supports 6-tuple padding (specific per side)
  • Handles both image and mask padding with separate fill values
albumentations/augmentations/transforms3d/transforms.py
Created BasePad3D class to share common padding functionality
  • Extracted common padding logic from PadIfNeeded3D
  • Implemented shared apply_to_images and apply_to_masks methods
  • Added common initialization parameters for fill values
albumentations/augmentations/transforms3d/transforms.py
Updated deprecation warnings implementation
  • Replaced Field(deprecated=...) with explicit warning calls
  • Updated warning messages to use DeprecationWarning type
  • Added proper stacklevel to warning calls
albumentations/augmentations/crops/transforms.py
albumentations/augmentations/geometric/transforms.py
albumentations/augmentations/geometric/rotate.py
Added comprehensive test coverage for new functionality
  • Added tests for different padding configurations
  • Added tests for fill value handling
  • Added tests for dtype preservation
  • Added tests for 2D-3D padding equivalence
tests/transforms3d/test_transforms.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @ternaus - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟡 Testing: 2 issues found
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

tests/transforms3d/test_transforms.py Show resolved Hide resolved
tests/transforms3d/test_transforms.py Show resolved Hide resolved
tests/transforms3d/test_transforms.py Show resolved Hide resolved
tests/transforms3d/test_transforms.py Show resolved Hide resolved
tests/transforms3d/test_transforms.py Show resolved Hide resolved
@ternaus ternaus merged commit 783dabb into main Dec 13, 2024
14 checks passed
@ternaus ternaus deleted the add_pad branch December 13, 2024 23:44
Copy link

codecov bot commented Dec 14, 2024

Codecov Report

Attention: Patch coverage is 80.26316% with 15 lines in your changes missing coverage. Please review.

Project coverage is 89.27%. Comparing base (b1a79c2) to head (7174f3e).
Report is 311 commits behind head on main.

Files with missing lines Patch % Lines
albumentations/augmentations/crops/transforms.py 50.00% 9 Missing ⚠️
...bumentations/augmentations/geometric/transforms.py 70.00% 3 Missing ⚠️
albumentations/augmentations/geometric/rotate.py 66.66% 2 Missing ⚠️
...entations/augmentations/transforms3d/transforms.py 97.61% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           main    #2197       +/-   ##
=========================================
+ Coverage      0   89.27%   +89.27%     
=========================================
  Files         0       50       +50     
  Lines         0     8644     +8644     
=========================================
+ Hits          0     7717     +7717     
- Misses        0      927      +927     

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

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