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

Official Python 3 support #1580

Closed
declension opened this issue Apr 16, 2015 · 46 comments

Comments

@declension
Copy link
Member

commented Apr 16, 2015

With Debian's recent announcement, it seems as good a time as ever to bring up the spectre of Python 3 again.

Things have come on a bit but I can't remember whatever happened to the notes we had for dependencies and blockers for a Py3K version of QL (PyGTK is long done now at least), and there don't seem to be any open issues to track this.

Whilst the QL Python 3 FAQ entry states there are no plans (though that was written some time ago), if there was a plan, it should probably be maintained here now...

@declension declension added this to the long-term milestone Apr 16, 2015

@declension declension added the python3 label Apr 16, 2015

@lazka

This comment has been minimized.

Copy link
Member

commented Apr 16, 2015

I think all our (important) dependencies support Python 3 now. I'm for moving everything to a Python2/3 code base as a start and target 3.4/5, as anything older will be irrelevant once we are done.

First thing to do is to figure out if the pickle file can be loaded somehow in Python3 (never tried, too scared...)

I'm not looking forward to all this.

@declension

This comment has been minimized.

Copy link
Member Author

commented Apr 17, 2015

Sounds like a plan. As usual I'd forgotten some of these complexities (pickle - eek).

I didn't mean to imply there's any urgency (hell they're talking about 2020)... just that it is happening at some point and, well at least now there's a conversation / tracker for it :)

@CreamyCookie

This comment has been minimized.

Copy link
Collaborator

commented Aug 1, 2015

@lazka I'm just curious, but what are the reasons for supporting both Python 2 and 3? Wouldn't the code be much cleaner if it wouldn't need to be backwards compatible?

@akdor1154

This comment has been minimized.

Copy link

commented Aug 1, 2015

jumped in out of curiosity at https://github.com/akdor1154/quodlibet/tree/python3.
2to3 does a pretty good job, apparently! There were a couple of things to fix wrt strings vs. bytes, but I got the UI showing reasonably easily. I'll continue to muck with it.

@lazka

This comment has been minimized.

Copy link
Member

commented Aug 1, 2015

@CreamyCookie It allows for gradual change instead of having one switch followed by bugs. And after the test suite runs under both python2 and python3 we could be a lot more confident that things continue to work as before..

@akdor1154 good to know, thanks

@akdor1154

This comment has been minimized.

Copy link

commented Aug 2, 2015

for testing, it would be great if you could package python3-mutagen in your Ubuntu ppa :)

@lazka

This comment has been minimized.

Copy link
Member

commented Aug 5, 2015

@akdor1154 python3-mutagen should be there now

@akdor1154

This comment has been minimized.

Copy link

commented Aug 5, 2015

legend!

@akdor1154

This comment has been minimized.

Copy link

commented Aug 6, 2015

Plan of attack: QL-py3 is in a state now where it at least requires a stern glare to make it crash, instead of a slight breeze (to play music while I work, it's fine). I'll work on getting all tests to pass, with as few changes to the tests themselves as possible, and these in separate commits (The global str/bytes overhaul is going to mean some tests need a bit of massaging). My general approach is that all representations inside QL should be Strings unless we are interfacing with the outside world, which thanks to Mutagen etc, QL doesn't actually do much of (Question: is this approach OK?).
To limit myself in scope (and because my understanding of the library pickling isn't going to be good enough), I am not going to touch library imports from old versions. In the meantime, I've changed QL to use a prefs folder called .quodlibet-py3 instead of .quodlibet, so users' libraries should be safe if they want to play around.
Is this work worth doing? I am happy to do it, but if people who are actually involved in the community are going to want to do it themselves then I'll step back. I'm sure everyone's intentions are good, but at the same time I don't really want to get into a situation where a month of my work gets rejected.
Cheers
Jarrad

@lazka

This comment has been minimized.

Copy link
Member

commented Aug 7, 2015

Quick summary of all the string types:

File Paths:

  • py2: unicode on windows, str everywhere else
  • py3: str with surrogateescape everywhere or possibly bytes on posix
    (Note: PyGObject support is a bit buggy under py3 [0])

Tag Keys:

  • py2: ascii str or unicode
  • py3: str

Tag Values:

  • py2: utf-8 str or unicode
  • py3: str

Mutagen:

  • py2: unicode mostly
  • py3: str mostly

Everything else should probably be str in py3 and unicode in py2, even if it is
str now due to laziness.

[0] https://bugzilla.gnome.org/show_bug.cgi?id=746564

@lazka

This comment has been minimized.

Copy link
Member

commented Aug 7, 2015

Using ".quodlibet-py3" sounds like a good idea.

As I said before I prefer a py2/3 codebase similar to how mutagen handles it: https://bitbucket.org/lazka/mutagen/src/default/mutagen/_compat.py

The porting task can be easily split into small tasks:

  1. get tests running
  2. fix some tests
  3. fix some more tests
  4. fix more to get UI up at least
  5. ...
@davidovitch

This comment has been minimized.

Copy link

commented Aug 7, 2015

If you want some more inspiration, and following a similar approach compared to mutagen: checkout the Six library. An other example can be found in a rather large and complex project Pandas, their compat module is here.

Since all the major projects in the scientific Python scene (Numpy, Scipy, Matplotib, Pandas, Cython, etc) are supporting PY2 and PY3 with a single code base, I think it safe to say this is a proven, sensible and robust way forward.

Thanks for providing Quod Libet, just discovered it today and it rocks!

@akdor1154

This comment has been minimized.

Copy link

commented Aug 8, 2015

Requiring Python 2 compatibility increases the complexity of the codebase noticabley. Python3 only is actually decreasing it in quite a few places. What are the benefits of supporting Python 2? (I'm sure they're there, I'm just trying to understand from your point of view!) I can understand that a library like mutagen wants to be usable by both py2 and py3 applications, but QL itself I don't think would have this consideration.
EDIT: perhaps I'm exaggerating on the increased complexity front - what I mean is that unicode strings everywhere leads to much less .decode and .encode lying around everywhere.

@akdor1154

This comment has been minimized.

Copy link

commented Aug 8, 2015

Paths question: Python3 recommends using bytes for paths on Unix platforms. There are a few bugs in projects I've seen from people assuming everyone uses UTF-8, so this is probably necessary. However, internally, QL uses a mix of strings and bytes (e.g. even get_user_dir returns a str at all times). This is then often joined with a string, e.g. os.path.join(get_user_dir(), 'config'). Of course, if get_user_dir() is changed to return an fsnative(path), this join will then fail as we are joining bytes and str.
I propose implementing a path_join(*paths) function in util.path that runs map(fsnative) on all paths, then os.path.join-s the result, and then using this everywhere instead of os.path.join. This means we don't need to do gross things like os.path.join(get_user_dir(), fsnative('config')) everywhere. However, this is still a pretty invasive change, so is this an acceptable approach?

@davidovitch

This comment has been minimized.

Copy link

commented Aug 8, 2015

I guess you have a point on questioning why QL should be compatible for both. The modules I usually work with are libraries, and a lot of other projects depend on it (PY2 and PY3). In that context 2/3 support is important. Maybe QL is mainly used as an application? If parts of QL can/could be used as a library then a Python 2/3 compatible code base makes sense.

I have not converted a large code base from 2 to 3 yet, but I could imagine that requiring 2/3 compatibility forces you to make sure you're not cutting too many corners in the conversion process. It will reveal explicitly the differences between PY2 and PY3, maybe that is a good thing?

@lazka

This comment has been minimized.

Copy link
Member

commented Aug 8, 2015

Requiring Python 2 compatibility increases the complexity of the codebase noticabley. Python3 only is actually decreasing it in quite a few places. What are the benefits of supporting Python 2? (I'm sure they're there, I'm just trying to understand from your point of view!) I can understand that a library like mutagen wants to be usable by both py2 and py3 applications, but QL itself I don't think would have this consideration.
EDIT: perhaps I'm exaggerating on the increased complexity front - what I mean is that unicode strings everywhere leads to much less .decode and .encode lying around everywhere.

  • It's more work to maintain two branches while adding features (the pygobject
    port was a pain, but there were too many changes to allow a mixed code base)
  • If for some reason the py3 work stops, the work done until then isn't lost.
  • Delaying the py3 switch is easier. With py3only it would mean extra work
    as the branches would diverge or need to be kept in sync, with mixed we
    just keep on working as usual.
  • There is extra work needed to switch the Windows and OSX builds. This
    allows to keep them on py2 longer and not fall behind.
  • If it turns out that py2/3 is a stupid idea we can switch to py3 only easily.
@lazka

This comment has been minimized.

Copy link
Member

commented Aug 8, 2015

Paths question: Python3 recommends using bytes for paths on Unix platforms. There are a few bugs in projects I've seen from people assuming everyone uses UTF-8, so this is probably necessary.

Is bytes for Python3 really recommended? I though it was the other way around that str+surrogate is recommended but bytes still works.

However, internally, QL uses a mix of strings and bytes (e.g. even get_user_dir returns a str at all times). This is then often joined with a string, e.g. os.path.join(get_user_dir(), 'config'). Of course, if get_user_dir() is changed to return an fsnative(path), this join will then fail as we are joining bytes and str.

get_user_dir() returns unicode under Windows, so it already returns fsnative.. or do you mean under py3?

I propose implementing a path_join(*paths) function in util.path that runs map(fsnative) on all paths, then os.path.join-s the result, and then using this everywhere instead of os.path.join. This means we don't need to do gross things like os.path.join(get_user_dir(), fsnative('config')) everywhere. However, this is still a pretty invasive change, so is this an acceptable approach?

What about using str+surrogate on all platforms? That would make os.path.join(get_user_dir(), "foo") work everywhere.

@akdor1154

This comment has been minimized.

Copy link

commented Aug 8, 2015

Sorry, I did quite a bit of reading, but it's hard to work out what is outdated and what is current. The current Python documentation (https://docs.python.org/3/library/os.path.html) recommends bytes for Unix paths in the opening paragraph, whereas other things indeed say surrogate escaping works. (in my brief testing, it does work, but I am on a utf-8 fs-encoding so it's possible there are cases it doesn't)

@lazka

This comment has been minimized.

Copy link
Member

commented Aug 8, 2015

You can test using LANG=C python3

The os.path comment seems outdated to me, but maybe I'm missing something.
One sign that it is supposed to work is that the new pathlib module only works with str+surrogate and not bytes.

@akdor1154

This comment has been minimized.

Copy link

commented Aug 8, 2015

I think you're right; assuming there's no nonsense on Windows, I think that means we can deal with all paths as strings (Gtk is returning strings from its filename functions as well). Enticing...

@CreamyCookie

This comment has been minimized.

Copy link
Collaborator

commented Aug 11, 2015

I don't know if this would help you, but there's an amazing library in Python 3.4+, which also has been backported to 2.7: pathlib

Instead of os.path.join(get_user_dir(), 'config'), you can do something like this:

from pathlib import Path

USER_DIR_PATH = Path(get_user_dir())

print(USER_DIR_PATH / 'config')
# prints: /home/anon/.quodlibet/config
# or alternatively (if you don't like that notation)

print(USER_DIR_PATH.joinpath('playlists', 'relaxin'))
# prints: /home/anon/.quodlibet/playlists/relaxin

Of course it can do much more than that.

@jrobeson

This comment has been minimized.

Copy link

commented Aug 14, 2015

beets discussion on pathlib. It seems like they have similar ground to cover (minus the gtk issues) beetbox/beets#1409

@lazka

This comment has been minimized.

Copy link
Member

commented Sep 12, 2015

Pushed some things: 262f7e2

First obstacle: the Python config parser was switched to text only, which means we can no longer save paths there and old config files may not load.

@CreamyCookie

This comment has been minimized.

Copy link
Collaborator

commented Sep 13, 2015

Cool!
I don't know if this is of any help, but there is a module named future_builtins, that provides Python 3 versions of builtins in Python 2.7+.

And to get even more Python 3 behaviour in Python 2: (More information on that here)

from __future__ import division, absolute_import, \
                       print_function, unicode_literals
@zsau zsau referenced this issue Nov 2, 2015
@zsau

This comment has been minimized.

Copy link
Contributor

commented Nov 18, 2015

Some things I ran across while working on this:

  • In devices/__init__.py, there's a call to devices.sort(). Classes aren't sortable in Py3, but in order to replace this line I'd have to understand why these were getting sorted in the first place.
  • In util/__init__.py, the mode argument to atomic_save is getting ignored.
  • In config.py, Py3's ConfigParser is choking on default_max_plugin_invocations because it's a bare numeric value. Is it safe to make that a string?
@lazka

This comment has been minimized.

Copy link
Member

commented Nov 18, 2015

In devices/__init__.py, there's a call to devices.sort(). Classes aren't sortable in Py3, but in order to replace this line I'd have to understand why these were getting sorted in the first place.

My guess is to make the order deterministic. So sorting them by name instead should work.

In util/init.py, the mode argument to atomic_save is getting ignored.

oversight

In config.py, Py3's ConfigParser is choking on default_max_plugin_invocations because it's a bare numeric value. Is it safe to make that a string?

should be a string

@pfps

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2016

I fixed a couple of minor Python 3 problems (unescaped \ in strings, operator.div) locally.

However, loading the song database is proving problematic. I guess one way forward would be to not use any pickled data on conversion, but how would that work?

@CreamyCookie

This comment has been minimized.

Copy link
Collaborator

commented Feb 14, 2016

I have no idea if this is of any help, but there seems to be a way to load pickled data from Python 2 in Python 3: http://stackoverflow.com/questions/28218466/unpickling-a-python-2-object-with-python-3

@lazka

This comment has been minimized.

Copy link
Member

commented Feb 14, 2016

However, loading the song database is proving problematic. I guess one way forward would be to not use any pickled data on conversion, but how would that work?

There was a suggestion to use a different default config dir (like ~/.quodlibet_py3) until we figure this out.

@lazka

This comment has been minimized.

Copy link
Member

commented Feb 14, 2016

I've pushed some more things to get the tests running. There is an endless recursion at one point which I think is due to print_d() (I'll look into that..)

btw. there also is a travis-ci job for python3: e.g. https://travis-ci.org/quodlibet/quodlibet/jobs/109234439

@lazka

This comment has been minimized.

Copy link
Member

commented Feb 14, 2016

I have no idea if this of any help, but there seems to be a way to load pickled data from Python 2 in Python 3: http://stackoverflow.com/questions/28218466/unpickling-a-python-2-object-with-python-3

This looks promising, thanks.

@lazka

This comment has been minimized.

Copy link
Member

commented Feb 18, 2016

All tests run now (and mostly fail): https://s3.amazonaws.com/archive.travis-ci.org/jobs/110005259/log.txt (117 test failure(s) and 1587 test error(s) for 2517 tests)

@lazka

This comment has been minimized.

Copy link
Member

commented Jul 25, 2016

129 test failure(s) and 1167 test error(s) for 2734 tests

@lazka

This comment has been minimized.

Copy link
Member

commented Nov 29, 2016

All non-plugin tests pass now. But things are still crashy since we only have 60-70% coverage

@lazka

This comment has been minimized.

Copy link
Member

commented Nov 30, 2016

All tests pass now. I've made the travis job mandatory now, but if that turns out to be too much of a burden It can be reverted.

It's very unlikely that running with py3 will break your audio files, since that is handled by mutagen which supports py3 for quite some time now. All the config data like playlists, library stats might break on the other hand. So if you value that make a backup first. In theory switching back and forth between py2/py3 should work.

osx/win still needs a bit of work.

Related issues can be tracked/created with the python3 label: https://github.com/quodlibet/quodlibet/labels/python3

@lazka lazka closed this Nov 30, 2016

@lazka

This comment has been minimized.

Copy link
Member

commented Dec 19, 2016

There is now a "quodlibet-py3" package in the Ubuntu/Debian PPA.

@rpodgorny

This comment has been minimized.

Copy link

commented Dec 19, 2016

@lazka could you provide a link, please? i'm unable to find it (and i'm not that familiar with debian/ubuntu). thanks!

@NoahAndrews

This comment has been minimized.

Copy link

commented Dec 19, 2016

PPAs are like extra repositories that you have to install manually. Here's the one that contains the new quodlibet-py3 package. Once it's set up, you can just run apt install quodlibet-py3, like any other package.

@ArchangeGabriel

This comment has been minimized.

Copy link

commented May 20, 2017

@lazka Do you think that when building/installing only ExFalso, making py3 the default is a good idea? (Asking for the Arch package)

More generally, do you think distro should switch to py3? Maybe for package like quodlibet-git in AUR?

@lazka

This comment has been minimized.

Copy link
Member

commented May 20, 2017

Hard to say how stable the py3 version is. We currently have the py3 PPA variant for Ubuntu and running from source also defaults to py3. My plan was to collect sentry error reports through them for some time and when things calm down do the switch.

Switching quodlibet-git in AUR sounds like a good idea.

@lazka

This comment has been minimized.

Copy link
Member

commented May 20, 2017

Remaining Python3 issues atm:

  • macOS builder not fully ported
  • Startup is slower atm for large libraries because it tries to convert the database every time; needs a flag in the DB.
@ArchangeGabriel

This comment has been minimized.

Copy link

commented May 27, 2017

How does one build python 3 version from release tarball? I’ve tried running python setup.py install and then running exfalso, but failed with this error:

Traceback (most recent call last):
  File "/usr/bin/exfalso", line 15, in <module>
    sys.exit(main())
  File "/usr/lib/python3.6/site-packages/quodlibet/exfalso.py", line 27, in main
    quodlibet.init(config_file=config_file)
  File "/usr/lib/python3.6/site-packages/quodlibet/_init.py", line 55, in init
    init_cli(no_translations=no_translations, config_file=config_file)
  File "/usr/lib/python3.6/site-packages/quodlibet/_init.py", line 140, in init_cli
    _init_python()
  File "/usr/lib/python3.6/site-packages/quodlibet/_init.py", line 93, in _init_python
    MinVersions.PYTHON2.check(sys.version_info)
  File "/usr/lib/python3.6/site-packages/quodlibet/const.py", line 48, in check
    self.name, self, Version("", *version_tuple), message))
ImportError: Python2 2.7 required. 3.6.1.final.0 found.
@lazka

This comment has been minimized.

Copy link
Member

commented May 27, 2017

How does one build python 3 version from release tarball?

You can't atm. I added that check for release builds so distros don't package for Python 3 by accident.

You can run "setup.py sdist" on git master to get a tarball for testing.

@ArchangeGabriel

This comment has been minimized.

Copy link

commented May 27, 2017

@lazka

This comment has been minimized.

Copy link
Member

commented May 27, 2017

@ArchangeGabriel

This comment has been minimized.

Copy link

commented May 28, 2017

Yes, that’s what I did. And it worked obviously. :)

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