Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ViewBox.setLimits() doesn't trigger update if view range not within new limits #2415

Open
bbc131 opened this issue Sep 14, 2022 · 6 comments · May be fixed by #2540
Open

ViewBox.setLimits() doesn't trigger update if view range not within new limits #2415

bbc131 opened this issue Sep 14, 2022 · 6 comments · May be fixed by #2540
Labels
ViewBox

Comments

@bbc131
Copy link
Contributor

bbc131 commented Sep 14, 2022

Description

If you call for example ViewBox.setLimits(minXRange=limit) with limit being larger as the current visible range, the ViewBox doesn't update its view range. We are then in a state where the limits are not satisfied.
My expectation is, that the view range is updated immediatly with the call of setLimits().
Only after some other trigger, like updating the view range by ViewRange.setRange(), the view range suddenly changes and the limits are satisfied.
This can be seen as a workaround: Just call setRange with the current range.
This is behaviour seems to be the case for all eight limit parameters (xMin, ..., yMax, minXRange, ..., maxYRange)

Code to reproduce

Use the following mwe to check the current behaviour:
The width of the LinearRegionItem sets the minXRange limit of the right plot.
At the beginning it's all good, the region has a width of 10 and the right plot a range of 20.
Increasing the regions width to above 20 doesn't affect the right plot.
Now, try to pan the right plot just a little bit, this will trigger the limits and the plot will "jump" into a state where the limits are satisfied.

The expected behaviour can be seen, if the workaround is enabled (see the if 0 in the code).

import pyqtgraph as pg

app = pg.mkQApp()

win = pg.GraphicsLayoutWidget(show=True)
win.resize(1000,600)

p1 = win.addPlot(title="Region defines limit for other plot")
lr = pg.LinearRegionItem([0,10])
p1.addItem(lr)
p1.setXRange(0,50)

p2 = win.addPlot()
p2.setXRange(0,20)

def lr_updated():
    r = lr.getRegion()
    region_span = r[1] - r[0]
    print(f"p2.setLimits(minXRange={region_span})")
    p2.setLimits(minXRange=region_span)

    if 0:  # workaround
        p2.setRange(xRange=p2.viewRange()[0], padding=0.0)

lr.sigRegionChanged.connect(lr_updated)
lr_updated()

if __name__ == '__main__':
    pg.exec()

Suggested solution

I think the solution would be to extend ViewBox.updateViewRange() at line 1576.
Between for axis in [0, 1]: and if aspect is not False and ... one should compare viewRange against limits, minRng and maxRng and adjust viewRange accordingly.

That shouldn't be that much work, so if you agree with this suggestion, I might find time in the near future to come up with a PR.
@j9ac9k j9ac9k added the ViewBox label Oct 1, 2022
@aksy2512
Copy link
Contributor

Hi, Can I work on this issue

@j9ac9k
Copy link
Member

j9ac9k commented Nov 23, 2022

Hi, Can I work on this issue

Please, have at it! Thanks!

@aksy2512 aksy2512 linked a pull request Nov 26, 2022 that will close this issue
Open
@aksy2512
Copy link
Contributor

aksy2512 commented Dec 9, 2022

Hi, Can I work on this issue

Please, have at it! Thanks!

Hi, i created a Pr for this issue. Please take a look at it and tell me if any change is required?

@j9ac9k
Copy link
Member

j9ac9k commented Jan 19, 2023

@bbc131 Thanks for the detailed report, the code to reproduce and the suggested fix.

One thing that I'm still puzzled about is what should be the desired behavior in the case where minXRange is set to a larger value than the current view range, which end of the range should be adjusted? In #2540 (👋🏻 @aksy2512 ) the suggestion is to adjust the boundary (right limit if x-axis, top limit if y-axis), but I'm not convinced that's what's actually warranted... or should we adjust both ends of the boundary an equal amount (assuming we don't come across other limits)... or does it just not matter 😆

Do you have an opinion on this @bbc131 ?

@bbc131
Copy link
Contributor Author

bbc131 commented Jan 19, 2023

Hi @j9ac9k,

in short, I think it doesn't matter that much, which end you are changing in these cases. This is already a corner case I think.

The original code seems to change the lower and upper end of the axis symmetrically. (But only after you try to click and drag the view a bit.)
I think this would be the least invasive behaviour of the plot. But if this leads to ugly code, maybe difficult to maintain, I maybe would prefer the current suggestion of @aksy2512 and adjust just one end of the view.

Now, playing again with this things, I figured another issue which needs to be taken into account, when fixing this issue:
When adjusting the view range, to match the minXRange and maxXRange, one must not forget the xMin and xMax limits (or the y-axis counterparts). With the current state of #2540, it can happen, that the view range is extended on the upper end, such that it fulfills the minXRange but then starts to violating the xMax limit.

@j9ac9k
Copy link
Member

j9ac9k commented Jan 20, 2023

Hi @j9ac9k,

in short, I think it doesn't matter that much, which end you are changing in these cases. This is already a corner case I think.

The original code seems to change the lower and upper end of the axis symmetrically. (But only after you try to click and drag the view a bit.) I think this would be the least invasive behaviour of the plot. But if this leads to ugly code, maybe difficult to maintain, I maybe would prefer the current suggestion of @aksy2512 and adjust just one end of the view.

Now, playing again with this things, I figured another issue which needs to be taken into account, when fixing this issue: When adjusting the view range, to match the minXRange and maxXRange, one must not forget the xMin and xMax limits (or the y-axis counterparts). With the current state of #2540, it can happen, that the view range is extended on the upper end, such that it fulfills the minXRange but then starts to violating the xMax limit.

Yeah, I had thought about this too; as this calculation occurs after checking the other limits, so we have to make sure they're taken into account.

@bbc131 bbc131 mentioned this issue Aug 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ViewBox
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Change viewBox.py aksy2512/pyqtgraph
3 participants
@j9ac9k @bbc131 @aksy2512