Change viewBox.py

[aksy2512]

Detail the reasoning behind the code change. If there is an associated issue that this PR will resolve add

Fixes #2415

I wrote the following code inside the updateViewRange function

[bbc131]

Hi @aksy2512,
I just had a look at your PR and also tested it quickly.

It solves the problem partly ;)
I took my example from #2415 and observe the following:
Making the region larger correctly triggers the right plot at some point to increase its x-range.
But if you make the region than smaller again, this will decrease the right plots x-range also! This definitely shouldn't be the case.
I just looked around in the updateViewRange method and saw, that one might have to adjust self.state['targetRange']?!
I hadn't the time to look closer into it, this is just a wild guess.

Another remark on formatting:
I suggest you to use black [1] to get a nice formatting.
For example I don't like the brackets at the "if"s and black would also normalize all whitespaces.
[1] https://pypi.org/project/black/

[j9ac9k]

Hi @aksy2512

Sorry I haven't reviewed this yet. I had a long stint of being sick and this time of year it's difficult to allocate time for pyqtgraph work due to family time and so on.

generally speaking I'm not big on a bunch of if-statements like this, but if it works it works. ViewBox.py is a very sensitive to changes so even though the implementation looks right, I'm going to try and break this a few times.

I am fairly confident this PR (or a variant of this PR that fixes the same issue) will be merged before the next release.

[aksy2512]

@j9ac9k I have rewritten the code and used "for" loop instead of the if statements. Please have a look.
Thanks

[j9ac9k]

Can this line be removed and incorporate the logic added into the for loop above?

[j9ac9k]

Hi @aksy2512

Sorry for the really long wait for feedback... ViewBox.py is among the most fragile parts of the library, so naturally I gravitate towards avoiding changes to it unless absolutely necessary.

There is an issue I'm a bit hung up on that I posted in the channel, why should the second limit be adjusted and not the first limit as this PR does.

I was going to suggest adding a test, but I can see we already skip the viewbox limits/resize test already as it's not working, so that would not be a reasonable request for a newcomer to the library :)

[j9ac9k]

@bbc131 made a good point in this post here #2415 (comment)

They point out that the current logic in this PR makes no attempt to compare the computed upper bound of the range against xMax/yMax, so there could be a situation where the newly computed upper bound is outside of the specified limits. If that occurs, the lower bounds should likely be adjusted when the upper bound is placed at the xMax or yMax limit.