Fix q svg widget

[homosapien-lcy]

A temporary fix for issue #1020. Closes #1020.

[mdickinson]

@homosapien-lcy This PR appears to include commits from a different PR (#1019). Was that intentional?

[homosapien-lcy]

We need a fix that works with any Qt backend, not just with PySide6.

Also, please remove the unrelated changes from this PR.

Thanks for the comments. I opened another issue for this in the pyface https://app.zenhub.com/workspaces/ets-queue-priority-64022b3641e35b00166c1272/issues/gh/enthought/pyface/1235. This PR is supposed to be a quick fix

[homosapien-lcy]

@homosapien-lcy This PR appears to include commits from a different PR (#1019). Was that intentional?

Accidentally included those... Just removed

[mdickinson]

This PR is supposed to be a quick fix

Again, we need a fix that works with any Qt backend, not just with PySide6. It's important to think carefully about the potential impact of code changes.

Question: what happens with the Enable main branch if someone with PySide2 installed tries to run the static_image.py demo? What happens after the change proposed in this PR?

[homosapien-lcy]

This PR is supposed to be a quick fix

Again, we need a fix that works with any Qt backend, not just with PySide6. It's important to think carefully about the potential impact of code changes.

Question: what happens with the Enable main branch if someone with PySide2 installed tries to run the static_image.py demo? What happens after the change proposed in this PR?

I think that won't work since we are using pyside6 here. So should we change the import in pyface (https://github.com/enthought/pyface/blob/main/pyface/qt/QtSvg.py)? Change all imports from QtSvg to QtSvgWidgets

[mdickinson]

I think that won't work since we are using pyside6 here.

Yes, exactly! The example works before this change, but fails afterwards. So yes, this needs to be fixed in Pyface, but it needs to be done carefully and tested with the various Qt backends there.

Change all imports from QtSvg to QtSvgWidgets

That won't work, because for PySide2, QtSvg is the right place to import from.

[mdickinson]

@homosapien-lcy For this PR, I see two possible ways forward:

(1) Close the PR and wait for the next Pyface release (assuming that this has been fixed in Pyface) (2) You may be able to do a try / except on the imports in this PR: e.g., first try importing from pyface.qt.QtSvgWidgets, then import from pyface.qt.QtSvg if that fails.

For (2), it's not clear to me whether this would work or not; it would need to be tested with all the currently supported Enable backends.

[mdickinson]

all the currently supported Enable backends

Note that there's a separate discussion to be had here on which backends we want to support going forward. It may make sense to drop support for PySide2 and PyQt5 at this point. (I think Pyface has already done this.)

[homosapien-lcy]

@homosapien-lcy For this PR, I see two possible ways forward:

(1) Close the PR and wait for the next Pyface release (assuming that this has been fixed in Pyface) (2) You may be able to do a try / except on the imports in this PR: e.g., first try importing from pyface.qt.QtSvgWidgets, then import from pyface.qt.QtSvg if that fails.

For (2), it's not clear to me whether this would work or not; it would need to be tested with all the currently supported Enable backends.

Got it. I think solution 1 sounds reasonable. But I also push the change for solution 2 if some user needs a quick fix

[mdickinson]

This issue is now fixed in Pyface, and my understanding is that we're expecting a Pyface release fairly soon. I'd recommend closing this PR.

[corranwebster]

It may make sense to drop support for PySide2 and PyQt5 at this point. (I think Pyface has already done this.)

No, we'll probably keep PyQt5 and PySide2 around for a while (at least until we support PyQt6, I would think)