Skip to content

DOC: Include missing gain parameter in adjust_gamma equation#7763

Merged
stefanv merged 3 commits intoscikit-image:mainfrom
larrybradley:gamma-docs
May 2, 2025
Merged

DOC: Include missing gain parameter in adjust_gamma equation#7763
stefanv merged 3 commits intoscikit-image:mainfrom
larrybradley:gamma-docs

Conversation

@larrybradley
Copy link
Contributor

@larrybradley larrybradley commented Mar 26, 2025

Description

The adjust_gamma docstring is missing the gain parameter in the equation describing the transformation.

Checklist

Release note

For maintainers and optionally contributors, please refer to the instructions on how to document this PR for the release notes.

...

Schefflera-Arboricola
Schefflera-Arboricola approved these changes Apr 19, 2025
View reviewed changes
@larrybradley larrybradley force-pushed the gamma-docs branch 2 times, most recently from 602dec4 to 352d7fc Compare April 25, 2025 15:35
@stefanv
Copy link
Member

stefanv commented Apr 25, 2025

This is fine to go in for now; but for @scikit-image/core, did anyone investigate why gain is there? Seems a bit weird, since it again pushes the image out of 0-1 range.

@larrybradley larrybradley force-pushed the gamma-docs branch 2 times, most recently from d40efd2 to 30cc9e1 Compare May 1, 2025 02:32
@mkcor
Copy link
Member

mkcor commented May 2, 2025

This is fine to go in for now; but for @scikit-image/core, did anyone investigate why gain is there? Seems a bit weird, since it again pushes the image out of 0-1 range.

With or without gain (in other words, whether gain == 1 or not), the image is always pushed back to its original range -- which is not constrained to [0, 1]:

scikit-image/skimage/exposure/exposure.py

Lines 682 to 684 in e8a42ba

scale = float(dtype_limits(image, True)[1] - dtype_limits(image, True)[0])
out = (((image / scale) ** gamma) * scale * gain).astype(dtype)

I checked the Git blame, and it's been like this ever since the original implementation 4d1c334 by @ankit-maverick; the most recent change, i.e., 7acfb7c by @rfezzani, doesn't change that.

@mkcor mkcor added the 📄 type: Documentation Updates, fixes and additions to documentation label May 2, 2025
mkcor
mkcor reviewed May 2, 2025
View reviewed changes
Copy link
Member

@mkcor mkcor left a comment

Choose a reason for hiding this comment

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

Thanks for catching this, @larrybradley!

I have suggested further edits to the docstring, so we describe what's actually implemented.

While we're at it, may I ask you to update the doctest? I mean:

import skimage as ski
image = ski.util.img_as_float(ski.data.moon())

and so on. Thank you!

@larrybradley
Copy link
Contributor Author

larrybradley commented May 2, 2025 •
edited
Loading

@mkcor -- thanks for the review. I've implemented all your suggestions.

@mkcor
Copy link
Member

mkcor commented May 2, 2025

Great, @larrybradley! Do you know that you may apply suggested changes via GitHub's web interface, including several at once (see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/incorporating-feedback-in-your-pull-request)?

mkcor
mkcor reviewed May 2, 2025
View reviewed changes
@stefanv @mkcor
Apply suggestions by @mkcor
d3c88ed 
Co-authored-by: Marianne Corvellec <[email protected]>
@stefanv stefanv merged commit 1abe332 into scikit-image:main May 2, 2025
21 of 23 checks passed
@stefanv
Copy link
Member

stefanv commented May 2, 2025

Thanks @larrybradley!

@stefanv stefanv added this to the 0.26 milestone May 2, 2025
@larrybradley larrybradley deleted the gamma-docs branch May 2, 2025 22:42
matthew-brett added a commit to matthew-brett/scikit-image that referenced this pull request Jul 7, 2025
@matthew-brett
Merge remote-tracking branch 'origin/main' into refactor-fundamental
91f2233 
* origin/main:
  Deprecate estimate method in favor of class constructor (scikit-image#7771)
  Temporary fix for Visual Studio & Clang incompatibility in Windows image (scikit-image#7835)
  Address deprecations in Pillow 11.3 (scikit-image#7828)
  Remove unused & obsolete `legacy_datasets`, `legacy_registry` vars (scikit-image#7677)
  Draft migration guide for skimage2 (scikit-image#7785)
  Do not report failure in wheels sub-recipe (scikit-image#7806)
  Use consistent wording in property description. (scikit-image#7804)
  Add intensity_median to regionprops (scikit-image#7745)
  CI: Update pypa/gh-action-pypi-publish to v1.12.4 for attestations on PyPI (scikit-image#7793)
  Document output dtype for transform.resize. (scikit-image#7792)
  Use `cibuildwheel` to build WASM/Pyodide wheels for `scikit-image`, push nightlies to Anaconda.org (scikit-image#7440)
  DOC: Include missing gain parameter in adjust_gamma equation (scikit-image#7763)
  Temporarily pin to `pyodide-build==0.30.0`, and ensure that the correct xbuildenvs are used (scikit-image#7788)
  Deprecate old names & attributes in RegionProperties (scikit-image#7778)
  Pin JasonEtco/create-an-issue action to SHA for v2.9.2 (scikit-image#7787)
  Make doctest-plus work with spin (scikit-image#7786)
  Report failures on main via issue (scikit-image#7752)
  Use `workers` instead of alternate parameter names (scikit-image#7302)
  Fix f-string in otsu plot (scikit-image#7780)
  Further document use of regionprops function and fix terms. (scikit-image#7518)
matthew-brett added a commit to matthew-brett/scikit-image that referenced this pull request Jul 14, 2025
@matthew-brett
Merge remote-tracking branch 'origin/main' into doc-some-params
70b9faa 
* origin/main: (31 commits)
  Update import convention in certain gallery examples (scikit-image#7764)
  Refactor fundamental matrix scaling (scikit-image#7767)
  Add unit test for cval unequal to zero
  Forward  in _generic_edge_filter
  Remove superfluous mask argument from _generic_edge_filter
  Only report failure on main branch once
  Deprecate estimate method in favor of class constructor (scikit-image#7771)
  Temporary fix for Visual Studio & Clang incompatibility in Windows image (scikit-image#7835)
  Address deprecations in Pillow 11.3 (scikit-image#7828)
  Remove unused & obsolete `legacy_datasets`, `legacy_registry` vars (scikit-image#7677)
  Draft migration guide for skimage2 (scikit-image#7785)
  Do not report failure in wheels sub-recipe (scikit-image#7806)
  Use consistent wording in property description. (scikit-image#7804)
  Add intensity_median to regionprops (scikit-image#7745)
  CI: Update pypa/gh-action-pypi-publish to v1.12.4 for attestations on PyPI (scikit-image#7793)
  Document output dtype for transform.resize. (scikit-image#7792)
  Use `cibuildwheel` to build WASM/Pyodide wheels for `scikit-image`, push nightlies to Anaconda.org (scikit-image#7440)
  DOC: Include missing gain parameter in adjust_gamma equation (scikit-image#7763)
  Temporarily pin to `pyodide-build==0.30.0`, and ensure that the correct xbuildenvs are used (scikit-image#7788)
  Deprecate old names & attributes in RegionProperties (scikit-image#7778)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mkcor mkcor mkcor left review comments

@stefanv stefanv stefanv approved these changes

+1 more reviewer

@Schefflera-Arboricola Schefflera-Arboricola Schefflera-Arboricola approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

📄 type: Documentation Updates, fixes and additions to documentation

Projects

None yet

Milestone

0.26

Development

Successfully merging this pull request may close these issues.

4 participants

@larrybradley @stefanv @mkcor @Schefflera-Arboricola