Skip to content

Conversation

@wli51
Copy link
Collaborator

@wli51 wli51 commented Sep 10, 2025

TL;DR
Adds bounding-box schema + image cropping utilities (with unit tests) as part of the ongoing virtual_stain_flow.datasets refactor.

PR Description
This PR is part 2a of the virtual_stain_flow.datasets refactor, which focuses on creating reproducible, logging-friendly datasets. The feature addition part is being split into smaller, stacked PRs to keep the review scope manageable.

This PR introduces support for cropping square/rectangular regions from raw microscopy images, along with supporting infrastructure.

Changes in this PR

  1. src/virtual_stain_flow/datasets/bbox_schema.py

    • Provides standardized formatting, validation, and access for crop metadata stored as bounding boxes in a DataFrame, so that:
    • dataset classes can focus on dataset-specific logic.
    • Essentially the realized dataset (not in this PR) can register the bounding box metadata with the accessor defined in this module and do self.bbox_accessor.coords(i) ;self.bbox_accessor.rot_centers(i);self.bbox_accessor.angle_of(i) to retrieve the parameters defining the ith crop.
  2. src/virtual_stain_flow/datasets/image_utils.py

    • Defines utilities for cropping image regions based on bounding boxes, and ensures:
    • image processing logic decoupled from dataset logic.
    • Essentially the realized dataset (not in this PR) can just call crop_and_rotate_image(image, ...) where ... are the bbox parameters retrieved from self.bbox_accessor and get the image properly cropped and rotated.
  3. Unit tests

    • Comprehensive tests for both modules (≈400–500 lines each) to ensure coverage and correctness.

Next Steps

  • A follow-up part 2b PR quick will add the realized dataset class and an example and unit test.

@gwaybio
Copy link
Member

gwaybio commented Oct 6, 2025

I'm planning on adding a link to this repo in a grant - can we merge this?

@gwaybio
Copy link
Member

gwaybio commented Oct 6, 2025

oh, I see, there's also a dev branch. Can we keep to trunk-based development?

@wli51
Copy link
Collaborator Author

wli51 commented Oct 6, 2025

I'm planning on adding a link to this repo in a grant - can we merge this?

Currently everything is on dev-0.1 which is essentially the main except it remained incomplete as a package until the very recent few PRs.

oh, I see, there's also a dev branch. Can we keep to trunk-based development?

I think this repo is already conforming to the trunk based development except the trunk points to a branch named dev-0.1. I think we can quickly merge the existing dev-0.1 to main.

Not sure if we want to merge this #14 without a review.

@gwaybio
Copy link
Member

gwaybio commented Oct 6, 2025

I think this repo is already conforming to the trunk based development except the trunk points to a branch named dev-0.1. I think we can quickly merge the existing dev-0.1 to main.

Sounds good, lets do this!

Not sure if we want to merge this #14 without a review.

Probably not... this is likely worth a bump (or a direct tag) in the pull request review channel. I also probably don't need to make the comment in this pull request. In other words, we can probably achieve dev-0.1 migration to main prior to merging this. My grant goes in next week

@wli51 wli51 changed the base branch from dev-0.1 to main October 6, 2025 21:28
@wli51
Copy link
Collaborator Author

wli51 commented Oct 6, 2025

I think this repo is already conforming to the trunk based development except the trunk points to a branch named dev-0.1. I think we can quickly merge the existing dev-0.1 to main.

Sounds good, lets do this!

Not sure if we want to merge this #14 without a review.

Probably not... this is likely worth a bump (or a direct tag) in the pull request review channel. I also probably don't need to make the comment in this pull request. In other words, we can probably achieve dev-0.1 migration to main prior to merging this. My grant goes in next week

Merged dev-0.1 to main and changed base of this PR to main. Should be good now. Please let me know if you want me to file a PR to brush up the main branch README to be more informative at root for the grant (I have planed to do it anyways, might as well do it now).

@gwaybio
Copy link
Member

gwaybio commented Oct 7, 2025

Thanks!

Please let me know if you want me to file a PR to brush up the main branch README to be more informative at root for the grant

This would be great :D

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