Confusing documentation for denoise_bilateral

[felipegb94]

Description

In the denoise_bilateral documentation the sigma_color description is a bit confusing when describing what img_as_float does in this function. It says that: Note, that the image will be converted using the img_as_float function and thus the standard deviation is in respect to the range [0, 1]

The above description is appropriate IF the input image has values of type int. So, in that case the user does need to take this transform into account when specifying sigma_color.

However, if the input image is of type float already, img_as_float does nothing. So, in that case the user should specify sigma_color based on the actual scale of the input image.

I think it might be important to clarify the second case I described so that users know how to specify sigma_color when their image is already of type float.

Link to denoise_bilateral docs

Link to denoise_bilateral code

Way to reproduce

import numpy as np
import skimage
img_float = np.ones((2,2))*5
img_uint8 = np.ones((2,2)).astype(np.uint8)*5
print(skimage.img_as_float(img_float ))
print(skimage.img_as_float(img_uint8 ))

Version information

# Paste the output of the following python commands
from __future__ import print_function
import sys; print(sys.version)
import platform; print(platform.platform())
import skimage; print("scikit-image version: {}".format(skimage.__version__))
import numpy; print("numpy version: {}".format(numpy.__version__))

3.6.10 | packaged by conda-forge | (default, Apr 24 2020, 16:44:11) 
[GCC 7.3.0]
Linux-4.4.0-19041-Microsoft-x86_64-with-debian-buster-sid
scikit-image version: 0.17.2
numpy version: 1.18.5

[grlee77]

However, if the input image is of type float already, img_as_float does nothing. So, in that case the user should specify sigma_color based on the actual scale of the input image.

Thanks for pointing this out. Indeed img_as_float used to automatically scale floats to [0, 1], but we have since moved away from that behavior across the library. Cases where default parameter values are only reasonable for floats in that range should be clarified via better documentation. I noticed the same thing recently for denoise_tv_chambolle and denoise_tv_bregman where the default weight setting is a reasonable starting point for floats in the range [0, 1], but may be substantially off otherwise.

Are you interested in making a documentation PR to address this for denoise_bilateral?

[mkcor]

Hi @felipegb94,

Sure! Would you like to submit a pull request to update the docs? The sentence is basically missing an if statement ("if the image is not floating point, ..."). You may want to point to https://scikit-image.org/docs/dev/user_guide/data_types.html.

Thanks for reporting!
Marianne

[felipegb94]

Sure I can try to submit a pull request.

I guess, according to the data types docs, denoise_bilateral expects floating point images to be between 0 and 1. But, what I think I am observing, is that denoise_bilateral still works fine if the image has float values outside the range 0 and 1.

[mkcor]

Sure I can try to submit a pull request.

Thank you for your PR!

I guess, according to the data types docs, denoise_bilateral expects floating point images to be between 0 and 1. But, what I think I am observing, is that denoise_bilateral still works fine if the image has float values outside the range 0 and 1.

Yes, functions taking float images as inputs assume that their range is -1 to 1. I think I can see what you mean, but I would need a real example to be sure.