Colormap upgrade cache #4371

payno · 2025-09-08T09:38:20Z

silx.gui.plot: ColormapMixIn: replace usage of a dict for the '__cacheColormapRange' by a LRU cache.

payno · 2025-09-08T12:54:10Z

src/silx/gui/plot/items/core.py

 class ColormapMixIn(_Colormappable, ItemMixInBase):
    """Mix-in class for items with colormap"""

+    COLORMAP_CACHE_SIZE = 25


The size is pretty arbitrary. Maybe it can be worth to increase it a bit.

Sounds good to me as it is, but you can increase it a bit if you want:
If you propose a slider with integer steps for the percentile, having COLORMAP_CACHE_SIZE = 128 would store all possible values.

Yes good point, lets go for 128.

payno · 2025-09-08T12:54:56Z

src/silx/gui/plot/items/core.py

            None if data is None else numpy.array(data, copy=copy or NP_OPTIONAL_COPY)
        )
-        self.__cacheColormapRange = {}  # Reset cache
+        self.__cacheColormapRange.clear()


I think using clear is clearer... without bad wordplay.

Clearer indeed!

src/silx/utils/test/test_cache.py

payno · 2025-09-10T06:33:40Z

src/silx/utils/cache.py

+from collections import OrderedDict
+
+
+class LRUCache:


Originally I wanted to inherit directly from OrderedDict. See this commit.
But it turned out that this implementation fails with python 3.10. Because the getitem API is used when calling move_to_end. So the appropriate way seems to have composition instead of inheritance. This forces me to expose the API...

Let's not add the LRUCache class to the public API for a single usage inside the library.
Then, best to implement only the method that gets used.
Also, since it is used only in one class and get is called at a single place, you could call move_to_end directly from the code if vRange is not None here (with a comment):

silx/src/silx/gui/plot/items/core.py

Lines 702 to 705 in bff0f83

vRange = self.__cacheColormapRange.get(key, None)

if vRange is None:

vRange = colormap._computeAutoscaleRange(data)

self.__cacheColormapRange[key] = vRange

👌 I moved the cache to silx.gui.plot.items._cache. After discussion we will keep the class for now as it also handle the limitation of the size and not only keeping the last element access at the end.

src/silx/utils/cache.py

t20100

IMO much simpler not to have a complete LRUCache (and maybe not at all).
Anyway, best to not put LRUCache in the API

t20100 · 2025-09-12T12:08:48Z

src/silx/gui/plot/items/core.py

            None if data is None else numpy.array(data, copy=copy or NP_OPTIONAL_COPY)
        )
-        self.__cacheColormapRange = {}  # Reset cache
+        self.__cacheColormapRange.clear()


Clearer indeed!

t20100 · 2025-09-12T12:22:36Z

src/silx/utils/cache.py

+from collections import OrderedDict
+
+
+class LRUCache:


Let's not add the LRUCache class to the public API for a single usage inside the library.
Then, best to implement only the method that gets used.
Also, since it is used only in one class and get is called at a single place, you could call move_to_end directly from the code if vRange is not None here (with a comment):

silx/src/silx/gui/plot/items/core.py

Lines 702 to 705 in bff0f83

vRange = self.__cacheColormapRange.get(key, None)

if vRange is None:

vRange = colormap._computeAutoscaleRange(data)

self.__cacheColormapRange[key] = vRange

t20100 · 2025-09-12T12:24:19Z

src/silx/gui/plot/items/core.py

 class ColormapMixIn(_Colormappable, ItemMixInBase):
    """Mix-in class for items with colormap"""

+    COLORMAP_CACHE_SIZE = 25


Sounds good to me as it is, but you can increase it a bit if you want:
If you propose a slider with integer steps for the percentile, having COLORMAP_CACHE_SIZE = 128 would store all possible values.

payno force-pushed the colormap_upgrade_cache branch 4 times, most recently from 010b974 to 1b78b29 Compare September 8, 2025 12:56

payno commented Sep 8, 2025

View reviewed changes

payno force-pushed the colormap_upgrade_cache branch from 1b78b29 to e00da8a Compare September 8, 2025 13:16

silx.utils: add 'LRUCache' class

870dcdd

payno force-pushed the colormap_upgrade_cache branch from e00da8a to 870dcdd Compare September 8, 2025 13:54

payno added 2 commits September 8, 2025 17:02

LRUCache: add work-around for python 3.10...

79f3f0c

LRUCache: update implementation to be compatible with python 3.10

f9d95ba

payno force-pushed the colormap_upgrade_cache branch from 5101584 to f9d95ba Compare September 10, 2025 05:04

payno commented Sep 10, 2025

View reviewed changes

src/silx/utils/cache.py Show resolved Hide resolved

payno mentioned this pull request Sep 10, 2025

colormap: allow users to provide percentile values for 'PERCENTILE' mode #4379

Merged

payno changed the title ~~DRAFT: Colormap upgrade cache~~ Colormap upgrade cache Sep 10, 2025

payno requested a review from t20100 September 10, 2025 09:36

t20100 reviewed Sep 12, 2025

View reviewed changes

payno added 3 commits September 12, 2025 14:56

LRUCache: reduce API scope

1b4679b

ColormapMixIn.COLORMAP_CACHE_SIZE: increase cache size.

894751c

Move src.silx.utils.cache to src.gui.plot.items._cache

03cb6da

payno force-pushed the colormap_upgrade_cache branch from bcbcc24 to 03cb6da Compare September 12, 2025 13:31

t20100 approved these changes Sep 16, 2025

View reviewed changes

payno merged commit 61f7098 into main Sep 16, 2025
4 checks passed

payno deleted the colormap_upgrade_cache branch September 16, 2025 14:52

	vRange = self.__cacheColormapRange.get(key, None)
	if vRange is None:
	vRange = colormap._computeAutoscaleRange(data)
	self.__cacheColormapRange[key] = vRange

Colormap upgrade cache #4371

Colormap upgrade cache #4371

Uh oh!

Conversation

payno commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

t20100 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

payno commented Sep 8, 2025 •

edited

Loading