MARDAScience / PBR_filter

{P}ansharpening by {B}ackground {R}emoval algorithm for sharpening RGB images
MIT License
4 stars 3 forks source link

Code Review #1

Open frank-engel-usgs opened 3 years ago

frank-engel-usgs commented 3 years ago

Purpose

This issue was created to serve as a review document for the USGS review of the PBR_filter repo.

Overview

I have reviewed and tested the code in the PBR_filter repository. The PBR_filter presents a novel method to pre-process images with a pansharpening-like approach. The resultant images will have increased contrast of edge features (especially) and differentiating color in transitions. The purpose is to aid in the detection of objects and training for Machine Learning (ML) applications, but would also be of use to other imagery science that often is looking for ways to increase contrast of texture or other features without changing the overall distribution of of values in the image.

Daniel has done a thorough job and the method appears to work on all tests I employed it on. I have no major recommendations to the code, however there were a few comments and minor suggestions which I have detailed in the next session.

Review

I followed the instructions in README.md to install using conda. Everything installed as expected (using Anaconda3 and PyCharms IDE).

That said, I was able to replicate all of the examples given in the readme. I also tried using the function on several examples of my own (typically UAS or stationary camera imagery used for Image Velocimetry). The approach is novel, or at least I have not seen it before, and I think I have a decent idea of the current state of literature for imagery manipulation. I can see several use cases:

Method Review

Code optimization / functionality /readability

I appreciate the inclusion of the warning suppression:

# rescale_sigma=True required to silence deprecation warnings
_denoise_wavelet = partial(denoise_wavelet, rescale_sigma=True)
from skimage import util

# import warnings filter
from warnings import simplefilter

However, I think it would be better to enable the warning messages. I could be convinced that leaving the partial(denoise_wavelet, rescale_sigma=True) makes sense, but not really the simplefilter application. This makes sense for a "one and done" script where users will not need any warnings. However, as I mention elsewhere in the review, I think this PBR_filter should be converted to not need tkinter, and be just callable as a class. In this case, the warnings may be more relevant.

Security / Other USGS stuff

frank-engel-usgs commented 3 years ago

@dbuscombe-usgs: I have completed my review of your PBR_filter. Interesting and useful tool! See my review in the comment above, and let me know if you have any comments or follow up questions.

dbuscombe-usgs commented 3 years ago

Thanks for the very thorough review. However this is not the code I asked you to review (dash doodler)!

frank-engel-usgs commented 3 years ago

Ha! Such is life. Ok, I'll review that one ASAP. Apologies. -F

dbuscombe-usgs commented 3 years ago

I'm on vacation now with bad signal for a few days but I will email when I can. Sorry for the confusion. It's doodler I would like to review and publish. See my original email with links to the code.usgs.gov repo. Thanks Frank!! 🙏

dbuscombe-usgs commented 3 years ago

So sorry for the confusion!! 😹

frank-engel-usgs commented 3 years ago

No sweat @dbuscombe-usgs: I was happy to review PBR_filter, as I see it as something I would likely use in the future. So, no loss.

I'm sorry this delays your expected review!