hi @gabrielebndn may I ask why you closed this PR?

Taking a glance at the CI failures, there appear to be two issues:

1. You're using f-strings, and while we plan on adopting NEP-29 soon (meaning we will be py3.7+) we're not there yet

>           return self.item.mapRectToDevice(self.item.boundingRect())
E           AttributeError: 'PySide2.QtWidgets.QGraphicsScene' object has no attribute 'mapRectToDevice'

I'm not familiar w/ the SVG exporter, I know there is another PR trying to fix it ( #1158 ); but let us know if you need help with it.

Hi @j9ac9k, thanks for your interest.

I closed this PR because I naïvely thought my fix would work out of the box, but after seeing the CI failing miserably I realized more work was needed to ensure compatibility with all supported versions. As I did not have the time to address it quickly, I didn't want to clutter the PR section until I had something reliable.

Yes, I had seen about the f-strings, that will be the easiest thing to fix :)
Concerning the second issue, I am really surprised, I don't have it on my system (Python3.6.9 + Qt5.15.1), but this may be due to the exporter being used in a different way than what I do, I'll have to investigate it. Any help on that would be super appreciated of course :)

Thanks for pointing out #1158, didn't know. I would say my PR seems slightly more mature and complete, but they have a similiar problem concerning an allegedly missing attribute.

I'll certainly come back to this PR and reopen it -- nights will be long in Paris under national curfew.

Per Github tutorial, a good time to open a PR is when you have enough of a code diff that you're ready to have a discussion:

https://guides.github.com/activities/hello-world/#pr

I would encourage you to re-open the issue, if we closed PRs every time our CI blew up, we wouldn't get anywhere 😆

When you have time, resolve the f-string issue and we can take a closer look at the other problem; ... I haven't looked at #1158 very closely at all, I only mentioned it in case it had addressed the issue you resolved. I have no preference for one PR vs. the other, I'll merge whichever one gets the CI to pass and resolved the issue.

Dear @j9ac9k, this PR does two things:

1. Fix width and height (which [WIP] SVGExporter: Write SVG viewbox size #1158 addresses)
2. Store background color too (which [WIP] SVGExporter: Write SVG viewbox size #1158 does not do)

Anyhow, I fixed the f-strings and reopened the PR. Sorry, I thought to do you guys a favour by closing an unready PR, but it turned out to be more annoying than just living it open 😅

Any comment is welcome

Hi @gabrielebndn leaving the PR open makes it a bit easier for us to hop onto the branch and experiment locally; ..thanks for re-opening. I do want to get this resolved, in part due to having numerous issues w/ the exporter, and having the other PR remain in WIP (and effectively having it go stale) ...would love to yank it off the clue (taking outstanding PRs to < 5 is a current goal).

I have a little time to look at this this evening; and first problem stuck out right away is here:

https://github.com/gabrielebndn/pyqtgraph/blob/topic/fixSVG/pyqtgraph/exporters/tests/test_svg.py#L36

This should be scene = pg.GraphicsScnee() but this yields a different issue w/ the test suite...

>       ex = pg.exporters.SVGExporter(scene)

pyqtgraph/exporters/tests/test_svg.py:77:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
pyqtgraph/exporters/SVGExporter.py:20: in __init__
    tr = self.getTargetRect()
pyqtgraph/exporters/Exporter.py:95: in getTargetRect
    return self.item.getViewWidget().rect()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <pyqtgraph.GraphicsScene.GraphicsScene.GraphicsScene object at 0x131a85820>

    def getViewWidget(self):
>       return self.views()[0]
E       IndexError: list index out of range

pyqtgraph/GraphicsScene/GraphicsScene.py:469: IndexError

Ok, ...I think it's working, but would you mind taking a closer look? I made a slight modification to test_simple I basically uncommented everything, and instead of declaring a scene explicitly, I created a GraphicsView, and then references the scene from that...

Here is my test_svg:test_simple method:

def test_simple():
    tempfilename = tempfile.NamedTemporaryFile(suffix='.svg').name
    print("using %s as a temporary file" % tempfilename)
    
    view = pg.GraphicsView()
    view.show()

    scene = view.sceneObj

    rect = pg.QtGui.QGraphicsRectItem(0, 0, 100, 100)
    scene.addItem(rect)
    rect.setPos(20,20)
    rect.translate(50, 50)
    rect.rotate(30)
    rect.scale(0.5, 0.5)
    
    rect1 = pg.QtGui.QGraphicsRectItem(0, 0, 100, 100)
    rect1.setParentItem(rect)
    rect1.setFlag(rect1.ItemIgnoresTransformations)
    rect1.setPos(20, 20)
    rect1.scale(2,2)
    
    el1 = pg.QtGui.QGraphicsEllipseItem(0, 0, 100, 100)
    el1.setParentItem(rect1)
    grp = pg.ItemGroup()
    grp.setParentItem(rect)
    grp.translate(200,0)
    grp.rotate(30)
    
    rect2 = pg.QtGui.QGraphicsRectItem(0, 0, 100, 25)
    rect2.setFlag(rect2.ItemClipsChildrenToShape)
    rect2.setParentItem(grp)
    rect2.setPos(0,25)
    rect2.rotate(30)
    el = pg.QtGui.QGraphicsEllipseItem(0, 0, 100, 50)
    el.translate(10,-5)
    el.scale(0.5,2)

    el.setParentItem(rect2)

    grp2 = pg.ItemGroup()
    scene.addItem(grp2)
    grp2.scale(100,100)

    rect3 = pg.QtGui.QGraphicsRectItem(0,0,2,2)
    rect3.setPen(pg.mkPen(width=1, cosmetic=False))
    grp2.addItem(rect3)

    ex = pg.exporters.SVGExporter(scene)
    ex.export(fileName=tempfilename)
    os.unlink(tempfilename)

Would you mind giving this a try and make sure it's working as intended? (keep in mind, there is a known issue w/ QGraphicsEllipseItem: #1095

Oh, cool! Thanks, I didn't know we had the right to modify the unit test, I thought it was made on purpose. Sure, I'll give it a try.

BTW, was you change made before or after 5bdd25e? Do you agree with that commit?

Sorry, my change was made before that commit; checking against QGraphicsScene didn't make sense to me, since

graphicsScene = pg.GraphicsScene()
baseGraphicsScene = QtGui.QGraphicsScene()
isinstance(graphicsScene, pg.GraphicsScene)  # True
isinstance(baseGraphicsScene, pg.GraphicsScene)  # False
isinstance(graphicsScene, QtGui.QGraphcsScene)  # True
isinstance(baseGraphicsScene, QtGui.QGraphiscScene)  # True

Within our library, we only reference QGraphicsScene for inheritance purposes (and apparently that test, no clue what that was all about).

Modifying tests is totally fair game; the objective here isn't just to make the test suite pass; but to have the library be functional and have the tests accurately test that functionality; so if your changes to Exporter.py and SVGExporter.py make it so SVG exporting works, but the test is failing, the next place to look is totally that test and make sure it's not bogus (which it appeared to be, but I should look at the commit that modified that test some more).

Hi @j9ac9k, I have reverted 5bdd25e and I have integrated your changes to this PR.

I have tested it locally with python3 and qt5, but I will admit this is the first time I delve into pyqtgrqph's source code and I have no idea on how to properly run the unit tests. I just launched it manually as ipython -i test_svg.py, and then from pyqtgraph import exporters; test_simple() within ipython (I had to do the manual import because ipython would not find the submodule otherwise, but I assume it's because I am not running the tests properly). It seems to work fine.

For reference, this is the image I obtained, don't know whether it corresponds to what is expected (converted to jpg because Github does not support svg 🙄 )

Feel free to join our slack channel; usually we try and be helpful for this kind of advice:

https://join.slack.com/t/pyqtgraph/shared_invite/zt-id2ko653-Ld~pQiIydeHKR9FnAApyJw

Regarding running tests, my preferred way of doing it is from the root directory `pytest -k "test_svg" Looks like you figured out some other ways of going about it.

I have no idea what the SVG output from the test is expected; I mostly meant in the context of the exporter, from your application, does the SVG output make sense (so for example, for the issue you were trying to fix for your use case, does it work?)

From my randomly exporting a couple of examples, it appears to be working for me.

If you can confirm it's working for you, I'll go ahead and merge. Thanks again!

I exported some SVGs and everything looked good, so I'm going to say this PR is good and merge. Thanks for the PR @gabrielebndn

I exported some SVGs and everything looked good, so I'm going to say this PR is good and merge. Thanks for the PR @gabrielebndn

Oh, excellent! Sorry for the late reply, glad this PR was appreciated :)