CampbellGroup / common

Shared campbell lab code.
GNU Lesser General Public License v3.0
8 stars 5 forks source link

replace pyqt4 with pyqt #155

Open jayich opened 7 years ago

jayich commented 7 years ago

qtpy gives us great flexibility. It works with qt4 or qt5. qt4 is at the point now where it is nontrivial to install in some cases (probably most).

gregllong commented 7 years ago

I think Andrew means qtpy

jayich commented 7 years ago

yes, corrected.

aransfor commented 7 years ago

according to https://github.com/spyder-ide/qtpy you still need to install qt4 and or qt5 so this does not help with the problem

jayich commented 7 years ago

The point here is that you can install qt4 or qt5 and the code will still work.

aransfor commented 7 years ago

I guess personally I would rather just switch to 5 eventually than have some intermediary thing

gregllong commented 7 years ago

The idea here is that since for our python 2.7 applications the Qt package that goes with the system is PyQt4, and for python 3.5 it's PyQt5 (I think this is true), it feels like backward compatibility is going to be necessary

jayich commented 7 years ago

Reading into pyqt I think we should at least switch to pyqt as an intermediate step before switching to pyqt5, if that is even desirable. pyqt5 has API changes from pyqt4, annoying. What is the argument for not using pyqt and letting the end user decide when/if they switch to pyqt5? or later likely pyqt6.

In any case, again I think it is better to incrementally move to pyqt5 through qtpy. Then when everyone is running pyqt5 we can discussing switching from qtpy to pyqt5. No one will notice the change from pyqt4 to qtpy except for running pip once. Not true for switching to pyqt5.

aransfor commented 7 years ago

I would prefer not to refactor the code twice.

jayich commented 7 years ago

The nice thing here is that you don't have to. @gregllong or I can knock this out.

aransfor commented 7 years ago

perhaps. I for see still having to test all the code I use to make sure it works and correcting things as they come up. This is always more time consuming than it seems

jayich commented 7 years ago

reading more carefully (here: https://pypi.python.org/pypi/QtPy) going to qtpy and then pyqt5 (if desirable) is the way to go. The switch from qtpy to pyqt5 is a replace all of qtpy with pyqt5.

I don't know how to lock step everyone upgrading their respective code bases to pyqt5. If we move to qtpy people can upgrade to pyqt5 whenever is convenient for their experiment.

jayich commented 7 years ago

A related issue: qt4reactor vs. qt5reactor. We should be able to do something like

try: 
    import qt4reactor as reactor
else: 
    import qt5reactor as reactor
jayich commented 7 years ago

For qt5reactor viability and easy pip installation it seems we are waiting on this PR.

Give the final comment a thumbs up.

aransfor commented 7 years ago

I mean I stand by what I said before that I think this has no clear benefit but will require everyone digging through there code... From my experience with "simple changes" to script scanner and the camera for instance it affected our experimental ability with only subjective organizational gains. That's my opinion FWIW

aransfor commented 7 years ago

Perhaps the real way to handle this stuff is to have a fork for refactoring changes like this and if you want to use that fork you can, plus those forks can merge periodically from common

jayich commented 7 years ago

@aransfor , I will look into qtpy and see if this is viable on a branch or fork.

crwood commented 7 years ago

For qt5reactor viability and easy pip installation it seems we are waiting on this PR.

Give the final comment a thumbs up.

I have recently taken over maintainership of the qt5reactor library and have since cut a new release (0.4) with the PR in question merged. I hope this will help ease your transition to Qt5!

jayich commented 7 years ago

@crwood thank you!

crwood commented 7 years ago

No problem! Please let me know if you come across any other outstanding issues along the way.

jayich commented 7 years ago

Anaconda no longer supports pyqt4. Would be great to have code that works with pyqt4 or pyqt5.

gregllong commented 7 years ago

A potential way of allowing for both pyqt4 and pyqt5 could be this. This requires the use of pyqtgraph which is not a problem for us

jayich commented 7 years ago

I'm not sure if we can do this, but if we can import the try/except statement so that we don't repeat that code switching between this method and potentially just pyqt4 or pyqt5 would be much easier. Not repeating this code should be possible.

aransfor commented 7 years ago

I vote for just switching to PyQt5 if were gonna do it then just get it done

jchristensen133 commented 7 years ago

I like the idea of switching to pyqt5. If the changes to the existing code are minimal, then I agree with @ransford. Can you guys comment on how much you think the existing code will need to be changed?

On Tue, Sep 12, 2017 at 10:59 AM, Anthony Ransford <notifications@github.com

wrote:

I vote for just switching to PyQt5 if were gonna do it then just get it done

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/CampbellGroup/common/issues/155#issuecomment-328933469, or mute the thread https://github.com/notifications/unsubscribe-auth/AFpkjpdbDyrBG7vZLARxv9CV8sCjrZjZks5shsZygaJpZM4Lp2oA .

aransfor commented 7 years ago

I just want to iterate that we should not maintain support for anaconda since this is just some specific package manager. However I think upgrading to pyqt5 is valuable in its own right and we should start a linger term branch that implements pyqt5

jayich commented 7 years ago

Regarding the amount of changes to existing code this is best figured out in a branch. The one big difficulty in switching to pyqt5 is changing other dependent repositories when common is ready. Just a number of moving pieces.

aransfor commented 7 years ago

@theoriginaljuice from looking at the pyqt5/pyqt4 differences there is a change to the signal and slots in pyqt5 as well as some changes to the Qdialog stuff. It looks like the former is used in scriptscanner quite a bit so it would have to be ported while as far as I know @gregllong is the only one who uses Qdialog so he can comment on how hard that change will be.

see here for differences http://pyqt.sourceforge.net/Docs/PyQt5/pyqt4_differences.html

aransfor commented 7 years ago

I agree with @jayich with other repos as well as scripscanner being the biggest issues, again without a branch (and a volunteer) we wont really know

jayich commented 7 years ago

Outside of Qdialog does getting scriptscanner working with pyqt5 cover all known pyqt usage? Getting scriptscanner working with pyqt5, test (on branch) and then implement the other pyqt5 changes through common might be an efficient route forward.

aransfor commented 7 years ago

I agree