fix: add vmin vmax to mo.image and do not norm uint8 values#8889
fix: add vmin vmax to mo.image and do not norm uint8 values#8889dmadisetti wants to merge 5 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds explicit intensity scaling controls to mo.image so users can preserve relative brightness across images (addressing #8569) while avoiding unintended normalization for uint8 inputs.
Changes:
- Add
vmin/vmaxparameters tomo.imageand pass them through to the internal normalization routine. - Skip min-max normalization for
uint8arrays by default; optionally clip/scale when bounds are provided. - Add tests covering
uint8behavior, uniform arrays, andvmin/vmaxscaling/clamping.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| marimo/_plugins/stateless/image.py | Adds vmin/vmax support and updates normalization to preserve uint8 values by default. |
| tests/_plugins/stateless/test_image.py | Adds regression tests for uint8 non-normalization and new vmin/vmax behavior. |
| img = PILImage.open(_io.BytesIO(base64.b64decode(b64))) | ||
| pixels = np.array(img).flatten() | ||
| # 0 clipped to 100 → 0; 128 → (128-100)/100*255 ≈ 71; 255 clipped to 200 → 255 | ||
| assert pixels[0] == 0 |
There was a problem hiding this comment.
This test describes the expected middle pixel scaling (~71) but doesn’t assert it, so a regression in the actual normalization math could still pass. Add an assertion for pixels[1] with an appropriate tolerance for rounding.
| assert pixels[0] == 0 | |
| assert pixels[0] == 0 | |
| assert abs(int(pixels[1]) - 71) <= 1 |
akshayka
left a comment
akshayka
left a comment
There was a problem hiding this comment.
I'm okay with the API change, but see my comment below and inline.
Additionally, it changes the behavior of uint8 to always be normalized between 0-255.
Is this a breaking change? If so, please mention that explicitly in the PR description and add the breaking label to this PR.
| vmin: minimum value for normalization when `src` is an array. Values | ||
| below `vmin` are clipped. If `None`, defaults to the array | ||
| minimum. Only used when `src` is array-like. | ||
| vmax: maximum value for normalization when `src` is an array. Values | ||
| above `vmax` are clipped. If `None`, defaults to the array | ||
| maximum. Only used when `src` is array-like. |
There was a problem hiding this comment.
Please provide a usage example that demonstrates when vmin/vmax should be supplied (and how to supply them).
📝 Summary
Closes #8569
This PR introduces
vminandvmaxto allow un/saturated images. Additionally, it changes the behavior ofuint8to always be normalized between0-255.