LegendItem: Enable customization of label text size and tests by dgoeries · Pull Request #1397 · pyqtgraph/pyqtgraph

dgoeries · 2020-10-13T13:50:03Z

This improves a little bit the customization of the LegendItem.

I use a default labelTextSize of 9pt at first. Suggestions?

dgoeries · 2020-10-13T15:53:35Z

Looks like I got trapped. The test is cracking locally even without changes to the LegendItem. I have to check

Rebase ...

j9ac9k · 2020-10-14T03:40:11Z

well, that's an odd test failure; only PyQt5 pipelines failing ....looks like something actually crashed during test_PolyLineROI

I'm able to replicate the issue locally:

pyqtgraph/graphicsItems/tests/test_InfiniteLine.py ..                                                                                                                                          [ 71%]
pyqtgraph/graphicsItems/tests/test_LegendItem.py .                                                                                                                                             [ 72%]
pyqtgraph/graphicsItems/tests/test_PlotCurveItem.py .                                                                                                                                          [ 72%]
pyqtgraph/graphicsItems/tests/test_PlotDataItem.py .....                                                                                                                                       [ 75%]
pyqtgraph/graphicsItems/tests/test_ROI.py ..zsh: abort      pytest

~/Developer/pyqtgraph legend-text-size-tests* 2m 1s

Running `pytest -k "test_ROI" does lead to an assertion failure...

j9ac9k · 2020-10-14T03:43:11Z

I'm getting this same failure on master, so a PR must have snuck in that's causing havok...

j9ac9k · 2020-10-14T04:26:19Z

I think I identified the issue in the library code, no clue how it went so long un-noticed! ...After I merge #1399, I'll re-run the pipeline (you may need to rebase your branch, I'm not sure...)

dgoeries · 2020-10-14T12:55:28Z

Thanks, but it is not fully fixed :(

It still crashes in the poly ROI test

j9ac9k · 2020-10-14T14:52:05Z

Thanks, but it is not fully fixed :(

It still crashes in the poly ROI test

This is soooo wired!

j9ac9k · 2020-10-14T15:52:55Z

well.... this is alarming:

pyqtgraph-pyqt5-latest ❯ pytest
========================================================== test session starts ===========================================================
platform darwin -- Python 3.8.3, pytest-5.4.3, py-1.9.0, pluggy-0.13.1
rootdir: /Users/ogi/Developer/pyqtgraph, inifile: pytest.ini
plugins: forked-1.2.0, cov-2.10.0, xdist-1.32.0
collected 159 items

examples/test_examples.py .....................s........................s...................                                       [ 41%]
pyqtgraph/dockarea/tests/test_dock.py ...                                                                                          [ 43%]
pyqtgraph/dockarea/tests/test_dockarea.py .                                                                                        [ 44%]
pyqtgraph/exporters/tests/test_csv.py .                                                                                            [ 44%]
pyqtgraph/exporters/tests/test_hdf5.py ...                                                                                         [ 46%]
pyqtgraph/exporters/tests/test_image.py .                                                                                          [ 47%]
pyqtgraph/exporters/tests/test_matplotlib.py ....                                                                                  [ 49%]
pyqtgraph/exporters/tests/test_svg.py ..                                                                                           [ 50%]
pyqtgraph/graphicsItems/PlotItem/tests/test_PlotItem.py ....                                                                       [ 53%]
pyqtgraph/graphicsItems/ViewBox/tests/test_ViewBox.py ..s                                                                          [ 55%]
pyqtgraph/graphicsItems/ViewBox/tests/test_ViewBoxZoom.py ........                                                                 [ 60%]
pyqtgraph/graphicsItems/tests/test_ArrowItem.py .                                                                                  [ 61%]
pyqtgraph/graphicsItems/tests/test_AxisItem.py .....                                                                               [ 64%]
pyqtgraph/graphicsItems/tests/test_ErrorBarItem.py .                                                                               [ 64%]
pyqtgraph/graphicsItems/tests/test_GraphicsItem.py ..                                                                              [ 66%]
pyqtgraph/graphicsItems/tests/test_ImageItem.py ...                                                                                [ 67%]
pyqtgraph/graphicsItems/tests/test_InfiniteLine.py ..                                                                              [ 69%]
pyqtgraph/graphicsItems/tests/test_LegendItem.py .                                                                                 [ 69%]
pyqtgraph/graphicsItems/tests/test_PlotCurveItem.py .                                                                              [ 70%]
pyqtgraph/graphicsItems/tests/test_PlotDataItem.py .....                                                                           [ 73%]
pyqtgraph/graphicsItems/tests/test_ROI.py ..zsh: abort      pytest

~/Developer/pyqtgraph legend-text-size-tests* 1m 56s
pyqtgraph-pyqt5-latest ❯ pytest -k "test_ROI"
========================================================== test session starts ===========================================================
platform darwin -- Python 3.8.3, pytest-5.4.3, py-1.9.0, pluggy-0.13.1
rootdir: /Users/ogi/Developer/pyqtgraph, inifile: pytest.ini
plugins: forked-1.2.0, cov-2.10.0, xdist-1.32.0
collected 159 items / 156 deselected / 3 selected

pyqtgraph/graphicsItems/tests/test_ROI.py ...                                                                                      [100%]

=================================================== 3 passed, 156 deselected in 4.35s ====================================================

~/Developer/pyqtgraph legend-text-size-tests*
pyqtgraph-pyqt5-latest ❯

j9ac9k · 2020-10-14T16:12:56Z

Following up a bit; looks like these two tests, when run together are triggering some kind of segfault

pyqtgraph-pyqt5-latest ❯ cat functions | gxargs --replace bash -c "pytest --cov pyqtgraph -k '{} or test_PolyLineROI' >& /dev/null || if [ $? = 139 ]; then echo {}; fi"

bash: line 1: 57717 Abort trap: 6           pytest --cov pyqtgraph -k 'test_legend_item_basics or test_PolyLineROI' >&/dev/null

I want to thank @asottile for showing me this debug method a little over a year ago in an unrelated issue.

Looking at your test more closely, you should not have this line:

app.deleteLater()

I commented out that line and the test suite ran fine for me.

dgoeries · 2020-10-15T08:25:29Z

Thanks for debugging my tests! Pipeline is passing now

pyqtgraph/graphicsItems/LegendItem.py

j9ac9k · 2020-10-15T16:09:07Z

Thanks for updating the PR @dgoeries those segfaults are really annoying to troubleshoot, I'm just happy the issue was straight forward to spot.

I added one more comment, that instead of having an assert check to make sure the right type, do an if statement, and perhaps emit a warning message and return None without attempting to modify the size. Alternatively you can just not do any checks and hope that whatever error handling Labeltem does would be sufficient (which I think would be fine too).

dgoeries · 2020-10-15T16:19:08Z

The error handling in the LabelItem is okay, it will throw. Changes were made, thanks for the review and help

j9ac9k · 2020-10-15T16:20:40Z

no problem, thanks for the PR, the build_docs stage will fail (for unrelated reasons), so don't worry about. Assuming everything else is good, I'll merge. Thanks again

j9ac9k · 2020-10-15T17:56:19Z

LGTM, thanks for the PR, merging.

dgoeries · 2020-10-15T23:03:27Z

Thanks, I will have two more PR's for the legend. :-)

Merge pull request #1 from pyqtgraph/master

cb0e159

Rebase ...

dgoeries force-pushed the legend-text-size-tests branch from fe9d035 to d1d5e39 Compare October 14, 2020 09:58

dgoeries added 2 commits October 14, 2020 12:01

Merge remote-tracking branch 'upstream/master'

166af15

LegendItem: Enable customization of label text size and tests

ac639ca

dgoeries force-pushed the legend-text-size-tests branch from d1d5e39 to ac639ca Compare October 14, 2020 10:03

One more CI

2be75be

Remove deleteLater of QAPP in legend item test

be10c72

j9ac9k reviewed Oct 15, 2020

View reviewed changes

pyqtgraph/graphicsItems/LegendItem.py Outdated Show resolved Hide resolved

Remove assert statement in setLabelTextSize

98a7f94

Modify legend test without assertion

a5b81db

j9ac9k merged commit b5de577 into pyqtgraph:master Oct 15, 2020

Uh oh!

Conversation

dgoeries commented Oct 13, 2020

Uh oh!

dgoeries commented Oct 13, 2020

Uh oh!

j9ac9k commented Oct 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

j9ac9k commented Oct 14, 2020

Uh oh!

j9ac9k commented Oct 14, 2020

Uh oh!

dgoeries commented Oct 14, 2020

Uh oh!

j9ac9k commented Oct 14, 2020

Uh oh!

j9ac9k commented Oct 14, 2020

Uh oh!

j9ac9k commented Oct 14, 2020

Uh oh!

dgoeries commented Oct 15, 2020

Uh oh!

Uh oh!

j9ac9k commented Oct 15, 2020

Uh oh!

dgoeries commented Oct 15, 2020

Uh oh!

j9ac9k commented Oct 15, 2020

Uh oh!

j9ac9k commented Oct 15, 2020

Uh oh!

dgoeries commented Oct 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

j9ac9k commented Oct 14, 2020 •

edited

Loading