Hi @klickverbot, your code looks much nicer. Can you give me an example (with minimal code if necessary) of the situation that is fixed by this? Ideally, you or I can write a unit test to cover that case.

My use case was just a vertical InfiniteLine with a label attached at position 0.95 as per the examples code. When setting an AxisItem.scale of, say, 1000 on only one of the left or bottom axis, the label would be placed off-screen.

I don't have access to the code right now, unfortunately, but the issue should be fairly easy to reverse-engineer – the problem in the original code is that the same px is used for both axes, which is incorrect with different scale factors involved. (In one of the directions, the label will still be shown near the line, as the offset will just be too small rather than too large.)

Unit tests for this would obviously be great, but I haven't had a chance to dig into setting them up for the graphical output yet.

Apologies, this is not related to AxisItem.scale at all, but just to the displayed ranges. I was manually pre-scaling the data to compensate for not setting .scale in the application where I found this issue which happened to more or less equalise the x and y ranges, hence the confusion. Commit message updated accordingly.

Here is a minimal test case:

win = pg.GraphicsLayoutWidget(show=True)

plot = win.addPlot()
plot.setXRange(0, 1)
plot.setYRange(0, 1e-3)

inf = pg.InfiniteLine(angle=90, label='test', labelOpts={'position':0.7})
inf.setPos(0.5)
plot.addItem(inf)

@campagnola: I added a unit test. It's somewhat pedestrian, but I didn't want the test to rely on the same local->scene coordinate conversion code.

Hmm, the test case passes locally (OS X); I've pushed a temporary commit to try and figure out what is going on.

Tests now pass; apparently there is just a bit over a pixel in placement difference between the platforms.

@campagnola: Sorry to be impatient, but: Ping? I'd like to avoid having to maintain an internal fork/monkey patch…

Codecov Report

Merging #727 into develop will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop     #727      +/-   ##
===========================================
+ Coverage    34.74%   34.75%   +<.01%     
===========================================
  Files          208      208              
  Lines        28285    28281       -4     
  Branches      4574     4572       -2     
===========================================
  Hits          9829     9829              
  Misses       17577    17577              
+ Partials       879      875       -4

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9aaae8d...46ddf61. Read the comment docs.

Rebased. (I also cleaned up the tests a bit a while back.)

@campagnola: Ping. The software I mentioned above that depends on this is going to be deployed more widely soon, and while having to pin a pyqtgraph Git commit instead of a release is one thing, shipping a custom fork is a bit annoying.

@dnadlinger Thanks for the PR, sorry we're taking a while to get to it, we're going through a lot of house-keeping and getting our test suite in order has been the priority.

I'll try and get to this in the coming few days; feel free to ping me if I haven't followed up in a week or so.

hi @dnadlinger, I attempted to do the conflict resolution and blew up your PR, sorry about that, I'll straighten this out.

So the CI system doesn't like this PR... failure pattern is the oddest I've ever seen. All Linux builds pass, all Windows builds fail, most macOS builds fail, but some pass. I'm on macOS and replicated one of the failed builds, and replicated the passed build as well.

I'll dive into this a bit more and see if I can turn anything up.

Now Windows images are failing on

>       assert not contains(hbr, 37, 185)
E       assert not True
E        +  where True = <function test_InfiniteLine_scaled_bounding_box.<locals>.contains at 0x0000018852A023A0>(<PyQt5.QtGui.QPolygonF object at 0x0000018852A00660>, 37, 185)

If I had more time I would get the custom pytest assertrepr functionality working so I could at least see what the bounding rect was for these polygons and see how close these points are...

CI is about to wrap up, two of the assert contains(...) checks are commented out. I'll see if anyone else has thoughts on how we should modify them (or remove those checks entirely).

No idea, sorry – I just modeled the tests after the existing ones.

Hi @dnadlinger figured as much due to thinking about how I would programmatically determine good points to verify. Wasn't expecting you to come back after 2 years, again, sorry its taken us so long to look into this. I'm still going to want to merge but I'll think a bit more about relevant test conditions.

Yep, it's a bit non-trivial, because ideally you wouldn't want to depend on the same piece of math used to compute the bounding box to verify the result as well.

I thought the scene coordinates should be quite stable, but perhaps there is a platform-dependent offset or something to take into consideration?

From my first re-run on this, it's not even platform dependent (which may just be a CI related issue, after all, I'm running these tests on headless systems with virtual displays, who knows how things render), but the test results varied within versions of Qt, so I'm not comfortable w/ even just experimenting until we find known good values, as things could change.

I do want to get this merged at some point, but I'm going to leave it open for now until I can try and come up with a more robust test method.

A simple regression test would just be to inspect _bounds – this would still show the problem with the old implementation.

And yes, experimenting is definitely not the way to go. I was just hoping someone who has the various coordinate transforms in pyqtgraph paged into their working memory could find a more appropriate coordinate space in which to do the comparisons.

Will be fixed in #2407 🎊