silx.gui.plot3d: Global size parameters now scales symbols with array sizes#4539
silx.gui.plot3d: Global size parameters now scales symbols with array sizes#4539
Conversation
| """Overrides supported ComplexMode""" | ||
|
|
||
|
|
||
| class SymbolMixIn(_SymbolMixIn): |
There was a problem hiding this comment.
Is it fine to implement it here in SymbolMixIn ? Or must I drill deeper in the hierarchy of classes?
There was a problem hiding this comment.
This is OK for me. I already hesitated to add the symbol size here as well instead of the base class in plot.
If we add multiple sizes support to 2d scatters, this will have to move to plot but that shouldn't break anything
t20100
left a comment
t20100
left a comment
There was a problem hiding this comment.
It is OK for me, but this adds a second way to set the symbol size. I'm wondering if it would not be best to have a different approach:
- Revert code to keep
get|setSymbolSizewith a unique value - Add
get|setSymbolSizeDatato pass an array of values or None to disable which gets normalized to range [1, getSymbolSize()] for display
What do you think?
| def __init__(self): | ||
| super().__init__() | ||
| self.__primitive = None | ||
| self.__scaleFactor = 1 |
There was a problem hiding this comment.
| self.__scaleFactor = 1 | |
| self.__scaleFactor = 1.0 |
| super().setSymbolSize(size, copy) | ||
| self._syncPointsPrimitive() | ||
|
|
||
| def setSizeScaleFactor(self, scaleFactor: float): |
There was a problem hiding this comment.
I propose to name it getSymbolSizeScaleFactor or getSymbolSizeScale. It's a bit long but makes it unambiguous which size it is.
Can you add a getter for it as well?
| """Overrides supported ComplexMode""" | ||
|
|
||
|
|
||
| class SymbolMixIn(_SymbolMixIn): |
There was a problem hiding this comment.
This is OK for me. I already hesitated to add the symbol size here as well instead of the base class in plot.
If we add multiple sizes support to 2d scatters, this will have to move to plot but that shouldn't break anything
That could work as well! The only problem is that the smallest marker will always have the size |
|
What about making |
I am okay if this does not go in the release. It is not essential and needs a rework as we discussed. |
2 participants
<Module or Topic>: <Action> <Summary>(see contributing guidelines)#4538 (comment)