Skip to content

Refactor crop generation module as subpackage to avoid single module expanding indefinitely with added features#32

Merged
wli51 merged 9 commits into
WayScience:mainfrom
wli51:refactor-crop-gen
Jul 21, 2026
Merged

Refactor crop generation module as subpackage to avoid single module expanding indefinitely with added features#32
wli51 merged 9 commits into
WayScience:mainfrom
wli51:refactor-crop-gen

Conversation

@wli51

@wli51 wli51 commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

The original src/virtual_stain_flow/datasets/ds_engine/crop_generator.py houses the protocol and two types of crop generating function. As more functionalities are added the single module would expand making it hard to read and maintain. So I decided to refactor the crop generation module as a sub package. Also added a new tiling crop generator.

wli51 added 3 commits June 10, 2026 09:34
…ated as a subpackage, moved crop generator protocol to its own stand aloen module and center and point-centered crop generator functions in their respective modules under the new subpackage.
…ubpackage while supporting older import signatures for compatibility

@MikeLippincott MikeLippincott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! Good PR. Please let me know if any of my comments/questions do not make sense.

Last thing here that should probably be added in this PR or another PR are tests for these functions/classes! Do they behave how you expect them to?

Comment thread src/virtual_stain_flow/datasets/ds_engine/crop_generators/center.py Outdated
Comment thread src/virtual_stain_flow/datasets/ds_engine/crop_generators/tile.py
Comment thread src/virtual_stain_flow/datasets/ds_engine/crop_generators/tile.py
@wli51

wli51 commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

Added tests.

@wli51

wli51 commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

@MikeLippincott thanks for the review. Addressed all your comments. Merging now!

@wli51
wli51 merged commit 96503bd into WayScience:main Jul 21, 2026
1 check passed
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