Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First try to adopt Python3 and PyQt5 #47

Merged
merged 36 commits into from Feb 21, 2019

Conversation

@Huluti
Copy link
Contributor

commented Nov 18, 2017

Here's my first bytes to adopt Python3 and PyQt5 in order to modernize this very useful application.
@Kilian Are you interested in this approach?

Here's the TODO that will be updated:

  • Find all possible regressions and missing changes.

I've also move the TODO file to the root directory for more visibility (standard practice) and convert it and the README files in markdown.

It seems to works quite well. I let you test these changes and give me advices. If you want to commit directly in my fork to update this PR, don't hesitate ;).

@Huluti Huluti force-pushed the Huluti:master branch from 837ddaf to fa9ff56 Nov 18, 2017

@Huluti Huluti force-pushed the Huluti:master branch from fa9ff56 to d7a0f84 Nov 18, 2017

@Huluti Huluti changed the title Not-ready: first try to adopt Python3 and PyQt5 First try to adopt Python3 and PyQt5 Nov 18, 2017

@Kilian

This comment has been minimized.

Copy link
Owner

commented Nov 21, 2017

Hey Huluti, What would you say are the main benefits of switching to py3 and pyQt5? It's been a while since I worked on Trimage so would like to err on the side of "if it ain't broken..."

@Kilian

This comment has been minimized.

Copy link
Owner

commented Nov 21, 2017

(aside from the obvious: we'll want to upgrade eventually, and it cleans up a whole bunch of code, nice!)

@Huluti

This comment has been minimized.

Copy link
Contributor Author

commented Nov 21, 2017

In my opinion, the best benefit is to ensure the existence of Trimage in the future when python 2 and qt 4 will be removed from the distribution directories. The work on the repo does not mean to release a new version, it can take some time before leaving an update to be sure of its quality. I am ready to help to make this possible.

@Huluti

This comment has been minimized.

Copy link
Contributor Author

commented Nov 24, 2017

Did that convince you?

@Kilian

This comment has been minimized.

Copy link
Owner

commented Dec 12, 2017

Will do!

@Huluti

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2018

@Kilian any news? ;)

@Kilian

This comment has been minimized.

Copy link
Owner

commented Mar 12, 2018

I've been busy outside of OS, sorry! I'll get to this in the near future, thanks for the reminder.

@Kilian

This comment has been minimized.

Copy link
Owner

commented Apr 8, 2018

I tried to run it. After getting the pyqt5 bindings for python 3, I quickly ran into this issue:

[kilian:...nal/trimage/python3/Trimage]$ python3 src/trimage/trimage.py                                                                                                                          
Traceback (most recent call last):
  File "src/trimage/trimage.py", line 18, in <module>
    from filesize import *
  File "/home/kilian/workspace/personal/trimage/python3/Trimage/src/trimage/filesize/__init__.py", line 1, in <module>
    from filesize import traditional, alternative, verbose, iec, si, size
ImportError: cannot import name 'traditional'
[kilian:...nal/trimage/python3/Trimage]$     

Does that work for you? I have very little experience with python3 (and haven't touch python in general in many years) so I have no clue on why this works in py2 and not in py3

@Huluti

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2018

I'm looking at it and I can not get it to work anymore ... I look at what I can do;)

@Huluti

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2018

Should be fixed ;)

@Kilian

This comment has been minimized.

Copy link
Owner

commented Apr 22, 2018

It starts, but there are three issues I see:

1 functionality is broken. We use generators and calling next changed, so this needs to be updated:

-- a/src/trimage/trimage.py
+++ b/src/trimage/trimage.py
@@ -171,8 +171,8 @@ class StartQT5(QMainWindow):
         delegatorlist = []
         for fullpath in images:
             try: # recompress images already in the list
-                image = (i.image for i in self.imagelist
-                    if i.image.fullpath == fullpath).__next__()
+                image = next(i.image for i in self.imagelist
+                    if i.image.fullpath == fullpath)
                 if image.compressed:
                     image.reset()
                     image.recompression = True

2 some filenames seems to give errors:

Traceback (most recent call last):
  File "src/trimage/trimage.py", line 334, in data
    data = self.imagelist[index.row()][index.column()]
  File "src/trimage/trimage.py", line 389, in __getitem__
    return self.d[key](self.image)
  File "src/trimage/trimage.py", line 357, in <lambda>
    'shortname': lambda i: self.statusStr() % i.shortname,
  1. In updating to pyqt5 for python3, it seems that the file dialog is now Qt's one instead of the native one. Can that be remedied?

After this lands, the debian folder needs an overdo as well, since from quick samples, it's no longer actually valid. Do you have experience there?

@Huluti

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2018

1 - So, what you've write is the correct solution that I have to add?

2 - What sort of filenames? Do you have examples?

3 - Strange because it seems that pyqt5 uses native dialogs by default...

I'm not an expert in packaging for debian but I can get a look 😃

@Mte90

This comment has been minimized.

Copy link

commented Dec 12, 2018

any updates? on debian they are planning to remove it because is not working anymore

@hosiet

This comment has been minimized.

Copy link

commented Feb 19, 2019

Just to let you know, trimage is listed on https://wiki.debian.org/Qt4Removal and we are killing Qt4-related applications one by one in Debian (as well as in Ubuntu).

@Huluti

This comment has been minimized.

Copy link
Contributor Author

commented Feb 20, 2019

I'll try to work on the PR today.

@Huluti

This comment has been minimized.

Copy link
Contributor Author

commented Feb 20, 2019

@Kilian for me all seems to work when launched with:

python3 src/trimage/trimage.py

It works for me with the setup.py too:

sudo python3 setup.py install

@Huluti Huluti force-pushed the Huluti:master branch from ccb0706 to 1e602b2 Feb 20, 2019

@Kilian

This comment has been minimized.

Copy link
Owner

commented Feb 21, 2019

Super nice @Huluti!

@hosiet could you assist us in updating the debian folder? (incrementing the version numbers is no problem, but there's probably some other notational changes that happened in the last 8 years that might require updates as well.

@Kilian Kilian merged commit 117a1a3 into Kilian:master Feb 21, 2019

@Huluti

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2019

@hosiet any thing to do to have Trimage 1.0.6 updated in Debian?

@hosiet

This comment has been minimized.

Copy link

commented Jun 9, 2019

First of all, Debian is now in deep freeze state preparing for the release of Debian 10; no new version could be uploaded at this moment. Secondly I'm not the maintainer of trimage in Debian; the maintainer is the team pkg-phototools-devel@lists.alioth.debian.org and Kilian Valkhof kilian@kilianvalkhof.com . I guess I'll contact them using email first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.