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

feat: conditional adaptive histogram equalization (CAHE) #427

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open

feat: conditional adaptive histogram equalization (CAHE) #427

wants to merge 30 commits into from

Conversation

cpaniaguam
Copy link
Member

@cpaniaguam cpaniaguam commented Aug 26, 2024 •
edited
Loading

Features

  • conditional_histeq and ancillary functions
  • Alternate implementation of annisotropic diffusion (AD) that is closer to MATLAB's

Other improvements

  • refactoring to the cloudmask workflow for code reusability. A part of the existing workflow is needed for CAHE
  • refactoring to the current implementation of AD

TODO

  • Validation tests for the new features

@review-notebook-app Review Notebook App
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@cpaniaguam cpaniaguam self-assigned this Sep 10, 2024
@cpaniaguam cpaniaguam changed the title chore: Add histogram equalization and anisotropic diffusion functions feat: conditional adaptive histogram equalization (CAHE) Sep 11, 2024
@cpaniaguam cpaniaguam added this to the IFT Data Process milestone Sep 11, 2024
@cpaniaguam cpaniaguam marked this pull request as ready for review September 11, 2024 20:12
tdivoll
tdivoll reviewed Sep 13, 2024
View reviewed changes
Copy link
Collaborator

@tdivoll tdivoll left a comment

Choose a reason for hiding this comment

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

Just some docstring cleanups/clarifications. Otherwise, it looks like good refactors and a few TODOs once we get clarifications from the research team.

src/cloudmask.jl Outdated Show resolved Hide resolved
src/cloudmask.jl Show resolved Hide resolved
src/histogram_equalization.jl Outdated Show resolved Hide resolved
src/histogram_equalization.jl Outdated Show resolved Hide resolved
src/histogram_equalization.jl Outdated Show resolved Hide resolved
src/histogram_equalization.jl Outdated Show resolved Hide resolved
src/histogram_equalization.jl Outdated Show resolved Hide resolved
src/histogram_equalization.jl Outdated Show resolved Hide resolved
@cpaniaguam
refactor: update function signatures and variable names for clarity
3b7ed15
@cpaniaguam
fix: update test assertions to use strict equality
727eca2
@cpaniaguam
fix: include histogram equalization module in IceFloeTracker
f22c4a2
@cpaniaguam
refactor: rename reference image parameter to false_color_image for c…
129b44c 
…onsistency
@cpaniaguam
refactor: rename getrgb_channels to get_rgb_channels for consistency
23f8e19
@cpaniaguam
fix: enhance package requirement parsing and import multiple skimage …
17bc887 
…modules
@cpaniaguam
refactor: reorganize module imports for improved clarity and structure
97f3f84
@cpaniaguam
test: add unit tests for cloud image workflow and adaptive histogram …
ca22f95 
…equalization
@cpaniaguam
test: add test data setup for conditional adaptive histogram equaliza…
ce9e629 
…tion
@cpaniaguam
test: add conditional adaptive histogram equalization test and refact…
ca890bb 
…or workflow tests
@cpaniaguam
fix: correct function name for RGB channel extraction in histogram eq…
243034f 
…ualization
@cpaniaguam
Copy link
Member Author

Regression tests added for these features.

danielmwatkins
danielmwatkins reviewed Sep 16, 2024
View reviewed changes
src/cloudmask.jl Outdated Show resolved Hide resolved
@cpaniaguam cpaniaguam linked an issue Sep 17, 2024 that may be closed by this pull request
Add entropy threshold before adaptive histogram equalization #425
Open
@cpaniaguam cpaniaguam mentioned this pull request Sep 17, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielmwatkins danielmwatkins danielmwatkins left review comments

@mirestrepo mirestrepo Awaiting requested review from mirestrepo

@hollandjg hollandjg Awaiting requested review from hollandjg

@digicosmos86 digicosmos86 Awaiting requested review from digicosmos86

@mkkim400 mkkim400 Awaiting requested review from mkkim400

@mmwilhelmus mmwilhelmus Awaiting requested review from mmwilhelmus

@tdivoll tdivoll Awaiting requested review from tdivoll

At least 1 approving review is required to merge this pull request.

Assignees

@cpaniaguam cpaniaguam

Labels
None yet
Projects
Ice Floe Coding Tasks
Status: In progress
Milestone
IFT Data Process
Development

Successfully merging this pull request may close these issues.

Add entropy threshold before adaptive histogram equalization
3 participants
@cpaniaguam @danielmwatkins @tdivoll