Hi @SimenZhor !

I am sure @j9ac9k will jump right in and have a closer look, but I wanted to comment on the limits / levels ambiguity:
ColorBarItem has an interactive level adjustment and IIRC the limits set the limits to this adjustment.

What you are looking for are the levels, as described in the setLevels() functionality of ImageItem. ImageItem.setImage() lets you set these through a keyword argument (...which is described in the docs for setOpts()...). That might be a good precedent, API-wise.

Do you think PColorMeshItem.setData() would benefit from also replicating the autoLevels parameter of ImageItem.setImage()? I believe that is a "keep what you have now" setting, and might avoid the need to specify the actual levels every time.

Otherwise this feature seems like a great addition, and indeed quite essential for some applications. Nice to see PColorMeshItem getting some attention!

Hi @NilsNemitz. Thanks for the encouraging response!

You are of course right about the kwarg naming. I'll make an update where I rename to levels.

Rewriting PColorMeshItem to operate like ImageItem with setOpts seems like a bigger job that I'm not sure if I'm suited for (as mentioned, this is my first PR here, and I'm not super familiar with the APIs in general).

I've played around a bit with the ImageItem class in order to understand the autoLevels parameter, and it seems like autoLevels is just a slightly more verbose way of doing what I'm trying to achieve with this PR. Quick examples based on pyqtgraph/examples/ImageItem.py:

# Ex1.1:
## Default behavior (autoscaling with each frame)
[...]
img = pg.ImageItem() # Setting levels here makes no difference to the end result in this example
[...]
def updateData():
    [...]
    img.setImage(data[i])

# Ex2.1:
## "Keep what you have" behavior
[...]
img = pg.ImageItem(levels=[1000,1500])
[...]
def updateData():
    [...]
    img.setImage(data[i], autoLevels=False) # autoLevels must be explicitly disabled for each frame

Corresponding examples for this PR:

# Ex1.2:
## Default behavior (autoscaling with each frame)
[...]
pcmi = pg.PColorMeshItem()
[...]
def updateData():
    [...]
    pcmi.setData(new_x, new_y, new_z)

# Ex2.2:
## "Set and forget" behavior
[...]
pcmi = pg.PColorMeshItem(levels=(-2,2))
[...]
def updateData():
    [...]
    pcmi.setData(new_x, new_y, new_z) # No need to explicitly disable autoscaling

If the autoLevels behavior is a well known and expected I'll gladly add it here, although I personally feel like it is slightly counter intuitive to continuously disable a setting.

I agree that continuously setting autoLevels=False is a little awkward.

In terms of intuition, I guess it depends on which direction you are coming from.
Many features in pyqtgraph are designed to improve
"Set the parameters once and then print."
to be
"Set the parameters once and then interact".

Sometimes the API design didn't quite make the leap to "Set the parameters at 100 FPS."
And now the exciting puzzle is to design around that!

Your comparison is awesome. How can we make both of these work the same way and still be intuitive and smooth for what you are trying to do? I don't think we want to have levels be persistent in one thing and not in another.

How about making your code work like

# Ex2.3:
## Compatible "Set and forget" behavior
[...]
pcmi = pg.PColorMeshItem(enableAutoLevels=False, levels=(-2,2))
[...]
def updateData():
    [...]
    pcmi.setData(new_x, new_y, new_z) # No need to explicitly disable autoscaling

Then we can port the enableAutoLevels keyword to ImageItem later and have less awkward updates for both.
This also nicely parallels the enableAutoRange keyword in ViewBox:
https://pyqtgraph.readthedocs.io/en/latest/api_reference/graphicsItems/viewbox.html#pyqtgraph.ViewBox.enableAutoRange
But that would just be my suggestion.

I think your suggestion sounds like a good middle ground. Should we wait for others to weigh in, or should I suggest an implementation right away?

@NilsNemitz, I've just pushed a suggestion for what the implementation of this behavior may look like. Let me know if you have any further comments or suggestions 😄

I made the improvements that were pointed out by @j9ac9k. Are there any next steps for me now, or just wait for more people to have the time to review my code? :)

Hi @SimenZhor I'm good w/ the PR as is, I'm going to leave it open in hopes to get the nod of agreement from @outofculture whose done a lot of the work with ImageItem if nobody brings up issues in a day or so, I'll merge 👍🏻

Thanks for the submission!

@outofculture I agree. That makes total sense. I've already started work on a new branch for a future PR where I'm planning to add ColorBar support as well to PColorMeshItem. In this usecase, the _updateDisplayWithCurrentState() method you describe has to be implemented (I've based my implementation on ImageItem, so the proof-of-concept commit I've made so far contains a lot of naming-bleed and completely unaltered docstrings - but you'll find the functionality you describe in updateImage() if you want to have a quick look).

I don't know if you prefer to wait for the work on that branch to be done or if we should just merge this one to keep the PRs shorter and cleaner. I suspect there will be a lot more discussion for getting the ColorBar support right. So, if possible, I'd prefer to do that in a different PR. I could of course include the setters in this PR, without doing full support for ColorBar. I'll leave that up to you.

I'm indifferent to that change being part of this PR vs a separate PR. I'll likely squash the PR on merge unless the change is quite massive, and if which case it is, I may ask (and I may be willing to help with) rebasing such that there are only a few commits that contain well-contained changes.

Sounds good, @j9ac9k. I'd still prefer to keep them separate though. I've barely started looking at the new feature, and if the consensus is that this PR is ok as-is (even though it could be improved) I'm happy to continue working on the other stuff at my own pace.

Sounds good to me, I'll merge as is. thanks for the PR @SimenZhor !