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.

Sign up for GitHub

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

Jump to bottom

Image Processing Class from PR 143 #171

Merged
merged 19 commits into from
Jun 19, 2024
Merged

Image Processing Class from PR 143 #171

merged 19 commits into from
Jun 19, 2024

Conversation

eloiseyang
Copy link
Collaborator

Combined functions in image_processing.py into ImageProcessor class.
Added ROI Classes for background subtraction and ROI cropping Added tests and datasets for both classes.
Separated from this pull request.

@phys-cgarnier @eloiseyang
Combined functions in image_processing.py into ImageProcessor class
6279d6e 
Added ROI Classes for background subtraction and ROI cropping
Added tests and datasets for both classes
Separated from physics-cgranier/lcls-tools/dev 8c85a56
@eloiseyang eloiseyang requested a review from nneveu June 12, 2024 23:28
@eloiseyang
Copy link
Collaborator Author

eloiseyang commented Jun 12, 2024 •
edited
Loading

TODO: Rename tests/datasets/h5py/test_image.h5 to clarify its contents.
If real beam image, change to fake Gaussian with known properties.

TODO: Run coverage.

TODO: Remove plotting from image class.

TODO: Rename image_processing folder to image.

TODO: Add ROI to docs

TODO: Cleanup Docs

eloiseyang
eloiseyang commented Jun 13, 2024
View reviewed changes
tests/datasets/h5py/test_image.h5 Outdated
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This file is not referenced in the contents of this PR. Maybe it's in the wrong place?

@eloiseyang
Copy link
Collaborator Author

Coverage looks good:

Name                                                                           Stmts   Miss  Cover
--------------------------------------------------------------------------------------------------
lcls_tools/__init__.py                                                             0      0   100%
lcls_tools/common/__init__.py                                                      0      0   100%
lcls_tools/common/image_processing/__init__.py                                     0      0   100%
lcls_tools/common/image_processing/image.py                                       30      1    97%
lcls_tools/common/image_processing/image_processing.py                            35      6    83%
lcls_tools/common/image_processing/roi.py                                         52      5    90%
tests/__init__.py                                                                  0      0   100%
tests/unit_tests/__init__.py                                                       0      0   100%
tests/unit_tests/lcls_tools/__init__.py                                            0      0   100%
tests/unit_tests/lcls_tools/common/__init__.py                                     0      0   100%
tests/unit_tests/lcls_tools/common/image_processing/__init__.py                    0      0   100%
tests/unit_tests/lcls_tools/common/image_processing/test_image.py                 32      3    91%
tests/unit_tests/lcls_tools/common/image_processing/test_image_processing.py      34      0   100%
tests/unit_tests/lcls_tools/common/image_processing/test_roi.py                   26      0   100%
--------------------------------------------------------------------------------------------------
TOTAL                                                                            209     15    93%

@MattKing06
Copy link
Collaborator

Added some comments but functionality looks good. Only other thing is to update the docs for image_processing to include ROI 👍

nneveu
nneveu reviewed Jun 17, 2024
View reviewed changes
Copy link
Member

@nneveu nneveu left a comment

Choose a reason for hiding this comment

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

Looks good, I left a few small comments/suggestions. Should be ready to merge soon.

docs/source/image.rst Show resolved Hide resolved
lcls_tools/common/image/image.py Outdated Show resolved Hide resolved
lcls_tools/common/image/processing.py Show resolved Hide resolved
lcls_tools/common/image/roi.py Outdated Show resolved Hide resolved
lcls_tools/common/image/roi.py Show resolved Hide resolved
lcls_tools/common/image/roi.py Outdated Show resolved Hide resolved
lcls_tools/common/matlab2py/mat_image.py Outdated Show resolved Hide resolved
tests/unit_tests/lcls_tools/common/image/test_processing.py Show resolved Hide resolved
nneveu
nneveu reviewed Jun 18, 2024
View reviewed changes
Copy link
Member

@nneveu nneveu left a comment

Choose a reason for hiding this comment

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

Will approve when checks passed, nice work! Thanks!

lcls_tools/common/image/roi.py Outdated Show resolved Hide resolved
nneveu
nneveu approved these changes Jun 19, 2024
View reviewed changes
Copy link
Member

@nneveu nneveu left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@eloiseyang eloiseyang merged commit 479c08c into slaclab:main Jun 19, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MattKing06 MattKing06 MattKing06 left review comments

@nneveu nneveu nneveu approved these changes

Assignees

@nneveu nneveu

@eloiseyang eloiseyang

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@eloiseyang @MattKing06 @nneveu @phys-cgarnier