Skip to content

Fix definition of entropy in NMI docstring#7750

Merged
jni merged 4 commits intoscikit-image:mainfrom
vcasellesb:fix-docstring-nmi
Mar 13, 2025
Merged

Fix definition of entropy in NMI docstring#7750
jni merged 4 commits intoscikit-image:mainfrom
vcasellesb:fix-docstring-nmi

Conversation

@vcasellesb
Copy link
Contributor

@vcasellesb vcasellesb commented Mar 12, 2025

See discussion at:

https://skimage.zulipchat.com/#narrow/channel/175598-general/topic/entropy.20formula.20.28in.20metrics.2Enormalized_mutual_information.29

Description

Hi! As commented on the linked topic from scikit-image's Zulip, I have changed NMI's docstring so that the entropy formula is better specified. This is just a documentation change, so I'm guessing no unit tests are required. However, I would recommend checking I haven't messed up using latex's math inside python, since I'm new to writing proper docs.

Release note

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

Fix the definition of entropy in the docstring of 
`skimage.metrics.normalized_mutual_information`. 

where :math:`H(X) := - \sum_{x \in X}{x \log x}` is the entropy.
where :math:`H(X) := - \sum_{x \in X}{p(x) \log p(x)}` is the entropy,
:math:`X` is the set of image values, and :math:`p(x)` is the probability
of observing a specific value :math:`x \in X`.
Copy link
Member

Choose a reason for hiding this comment

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

The only thing I'm nervous about here is the word "observing", which I think is overloaded. "encountering", or "x occurring in X"?

Copy link
Contributor Author

@vcasellesb vcasellesb Mar 12, 2025

Choose a reason for hiding this comment

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

I've changed it to:

`p(x)` is the probability of ocurrence of a specific value `x \in X`. 

Does that sound better? Thanks!

@stefanv
Copy link
Member

stefanv commented Mar 12, 2025

Thank you @vcasellesb!

@jni jni changed the title changed docstring in NMI definition (https://skimage.zulipchat.com/#n… Fix definition of entropy in NMI docstring Mar 13, 2025
@jni jni added the 📄 type: Documentation Updates, fixes and additions to documentation label Mar 13, 2025
@jni jni added this to the 0.26 milestone Mar 13, 2025
@jni
Copy link
Member

jni commented Mar 13, 2025

I pushed a minor typo and wording fix @vcasellesb (4d23f25), but imho this is ready now! Thank you!

@jni jni merged commit 660cdc9 into scikit-image:main Mar 13, 2025
24 of 25 checks passed
@jni
Copy link
Member

jni commented Mar 13, 2025

I checked the CI docs build (here) and everything looked good, so I merged. Thanks @vcasellesb!

@vcasellesb vcasellesb deleted the fix-docstring-nmi branch March 13, 2025 09:04
@vcasellesb
Copy link
Contributor Author

Thanks to you guys! @jni @stefanv

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📄 type: Documentation Updates, fixes and additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants