Move image utils function into lib/image_utils.py
#9725
Conversation
jrieke
changed the title
lib/image_utils.pylib/image_utils.py
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| from streamlit.errors import StreamlitAPIException | ||
| from streamlit.proto.Image_pb2 import ImageList as ImageListProto | ||
| from streamlit.runtime import caching | ||
| from streamlit.type_util import NumpyShape |
Check notice
Code scanning / CodeQL
Cyclic import
|
|
||
| import streamlit.elements.image as image_utils | ||
| from streamlit.deprecation_util import show_deprecation_warning | ||
| from streamlit.elements.lib.image_utils import WidthBehavior, marshall_images |
Check notice
Code scanning / CodeQL
Cyclic import
| from streamlit.delta_generator_singletons import get_dg_singleton_instance | ||
| from streamlit.elements.image import AtomicImage, WidthBehaviour, image_to_url | ||
| from streamlit.elements.lib.form_utils import is_in_form | ||
| from streamlit.elements.lib.image_utils import AtomicImage, WidthBehavior, image_to_url |
Check notice
Code scanning / CodeQL
Cyclic import
| from streamlit.elements.lib.image_utils import ( | ||
| Channels, | ||
| ImageFormatOrAuto, | ||
| ImageOrImageList, | ||
| WidthBehavior, | ||
| marshall_images, | ||
| ) |
Check notice
Code scanning / CodeQL
Cyclic import
jrieke
added
security-assessment-completed
| from typing import Any | ||
|
|
||
| import numpy.typing as npt | ||
| from PIL import GifImagePlugin, Image, ImageFile |
Check notice
Code scanning / CodeQL
Unused import
jrieke
changed the title
lib/image_utils.pylib/image_utils.py
| # `npt.NDArray[Any]` returns a `npt.NDArray[Any]`, so we need to | ||
| # ignore redundant casts below. | ||
| image_data = _np_array_to_bytes( | ||
| array=cast("npt.NDArray[Any]", image), # type: ignore[redundant-cast] |
There was a problem hiding this comment.
MyPy complained that the ignore statement here is unused, so I removed it (as well as the comment above) in image_utils.py. Not entirely sure though if this is still needed for some very old numpy versions?
There was a problem hiding this comment.
Yeah it looks like this was added 2 years ago in #4761, so I hope its safe to remove now.
Based on the comment it sounds like we might be able to remove the cast as well as it is described as redundant. Could you try this out and see whether anything complains?
There was a problem hiding this comment.
Removed the cast and everything seems to work fine, so I think it's probably fine to leave it like this. I can't imagine a realistic scenario where someone would use the latest Streamlit version but a NumPy version that's older than 2 years.
raethlein
left a comment
raethlein
left a comment
There was a problem hiding this comment.
LGTM, thanks for the refactoring!
## Describe your changes `lib/streamlit/elements/image.py` contained a bunch of utils functions that were imported in other files, which caused cyclic imports in streamlit#9711. This PR moves all of these functions into `lib/streamlit/elements/lib/image_utils.py` and makes all other files import directly from there. Unfortunately, this PR didn't remove all cyclic imports. But it's still worth merging this to clean up our codebase a bit more. ## GitHub Issue Link (if applicable) Just a refactor, no new functionality or tests added. ## Testing Plan --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license. --------- Co-authored-by: Johannes Rieke <[email protected]>
Describe your changes
lib/streamlit/elements/image.pycontained a bunch of utils functions that were imported in other files, which caused cyclic imports in #9711. This PR moves all of these functions intolib/streamlit/elements/lib/image_utils.pyand makes all other files import directly from there.Unfortunately, this PR didn't remove all cyclic imports. But it's still worth merging this to clean up our codebase a bit more.
GitHub Issue Link (if applicable)
Just a refactor, no new functionality or tests added.
Testing Plan
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.