Skip to content

Conversation

@flacjacket
Copy link

Still not quite working in Python 3, but tests still pass in Python 2. Still working on sorting out the last of the bugs, once I do, I'll clean up the commit history/messages. Needs python3-xlib, in addition to cffi dependencies.

@tych0
Copy link
Owner

tych0 commented Jun 25, 2014

Are we just depending on python3-xlib for the tests?

@flacjacket
Copy link
Author

Yeah, this doesn't need the super() calls, I removed that commit.

@flacjacket
Copy link
Author

Yeah, I think it just needs python3-xlib for tests, it's just so it can import Xlib

@flacjacket
Copy link
Author

Well, I think all the tests pass when I run them individually (using both my patches submitted for xcffib), but running the full test suite with Python 3 still generates some errors. But, nothing is broken with Python 2.7 at this point (2.6 might now have failures, since I've been putting in some things that I think may have only been backported from 3 to 2.7, and will need some compatibility layer or __future__ imports). I know a couple widgets still are broken in 3, namely the yahoo_weather and bitcoin_ticker, since urllib has changed in Python 3.

@tych0
Copy link
Owner

tych0 commented Jun 26, 2014

Ok, cool. Will try and take a look at these a bit later today and merge them.

@flacjacket
Copy link
Author

Alright, I can get all the tests passing in Python 3 now, and I've squashed everything down. There are still some oddities with the test suite, things seem to go a bit haywire if I don't do a find -iname '*.pyc' -exec rm {} \; when I switch between testing different Python versions, and every once in a while, Python 3 will throw this:

======================================================================
ERROR: test.test_manager.test_adddelgroup
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib64/python3.3/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/sean/qtile/qtile/test/utils.py", line 95, in wrapped_fun
    return function(self)
  File "/home/sean/qtile/qtile/test/test_manager.py", line 258, in test_adddelgroup
    self.c.delgroup(i)
  File "/home/sean/qtile/qtile/libqtile/command.py", line 113, in __call__
    return self.call(self.selectors, self.name, *args, **kwargs)
  File "/home/sean/qtile/qtile/libqtile/command.py", line 246, in call
    raise CommandError(val)
libqtile.command.CommandError: No such group: a

----------------------------------------------------------------------
Ran 163 tests in 103.682s

FAILED (errors=1)

but not every time.

@flacjacket
Copy link
Author

I'm not sure what if any problems stem from using python3-xlib to run the test suite, it doesn't seem to be well maintained and kept up with the current python-xlib resease. I might try to bring that up to par, I've already had to get one PR into that project to get this to run. When I get home, I'll try to get this up and running as my primary window manager.

@tych0
Copy link
Owner

tych0 commented Jun 26, 2014

On Thu, Jun 26, 2014 at 12:41:47PM -0700, Sean Vig wrote:

I'm not sure what if any problems stem from using python3-xlib to run the test suite, it doesn't seem to be well maintained and kept up with the current python-xlib resease. I might try to bring that up to par, I've already had to get one PR into that project to get this to run. When I get home, I'll try to get this up and running as my primary window manager.

Ok, cool. Another option would be to just rewrite whatever we have
that depends on xlib to use something else, I doubt it is very big if
it is in the tests.

xcffib still has a couple of bugs that might prevent it from actually
working as a primary window manager:

  1. the encoding of some requests (opcodes 6, 7, 8, 12, and 18 at
    least, based on my logs) is still (probably) broken. I have a patch
    to fix at least one encoding bug that I will push ASAP, but I'm
    sure there are others.
  2. Padding in offset calculations depending on bufsize xcffib#3 is still around. I've refactored things a bit so I
    think this can be done with some changes in Unpacker in
    init.py, but I haven't gotten around to actually looking at it
    yet. If qtile uses any messages which need special padding, those
    will be broken as well.

I know the first one at least prevents the systray from working, I'm
not sure about the second.

In any case, thanks a bunch for working on this. I'll try and review
in the next 24 hours!


Reply to this email directly or view it on GitHub:
#2 (comment)

@flacjacket flacjacket changed the title [WIP] qtile on Python 3 qtile on Python 3 Jun 27, 2014
tych0 added a commit that referenced this pull request Jun 28, 2014
@dequis
Copy link

dequis commented Jun 29, 2014

Hey, nobody told me this was happening!

Do we really want to avoid depending on six? I was looking into adding a metaclass today (didn't do it - found a simpler solution), and it turns out it's rather hard to keep that compatible in both versions, but six makes it simple. I just think that having our own compatibility layer might make things harder in the long term.

Also, what's the minimum python3 version we're aiming for?

@flacjacket
Copy link
Author

Ah, yeah, now that it's working, it may be good to mention it on the email list.

I'd forgotten about fixing the metaclass for Layout. The compatibility stuff I currently have in libqtile/compat.py [1], and is fairly minimal, I don't think we really need a full fledged six compatibility layer. We can add the with_metaclass method there (maybe with a license tag? six is also MIT licensed).

Currently, it's 3.3+, since it still has u"..." unicode strings, but it could easily go 3.2+ with an extra function to deal with the few unicode strings.

[1] https://github.com/flacjacket/qtile/blob/cffi-py3/libqtile/compat.py

@dequis
Copy link

dequis commented Jun 29, 2014

I'm okay with borrowing code from six!

And 3.3+ is perfectly okay IMO, since even 3.4 is out now (doesn't seem to have any interesting changes to help porting afaik).

@tych0
Copy link
Owner

tych0 commented Jun 29, 2014

Is there any reason to copy six in tree? xcffib depends on six, so users will have it installed anyway.

@dequis
Copy link

dequis commented Jun 29, 2014

...oh.

welp.

This value should be an integer (I think) and needs to be an integer to
be compared to `len(qtile.groups)` on libqtile/window.py L690.  Doing
this, however, introduces a problem with the Python 3 tests, and so it
always just returns `None`.
Make codebase compatible to run on Python 2 and 3.  Key changes include:

* Using Python 3 style absolute imports

* Use of Python 3 print function

* Proper handling of bytes/string/unicode for Python 2 and 3
  compatibility

* Using `//` for integer division

* Addition of a compatibility layer (libqtile.compat) for things that
  have been moved or renamed between 2 and 3
@flacjacket
Copy link
Author

The only reason is to drop a dependency, 2.6+ and 3.2+ support is fairly straightforward and requires a minimal (20 lines of code) compatibility layer (xcffib would be super easy to drop six from, since the lions' share of six calls are just BytesIO). This PR only needs the 9 lines of weird metaclass handling (which six actually took from Flask) to be complete, so if we can take those 9 lines and tack the license to them (?? I'm not too keen on licensing issues), that'd be fairly easy.

@tych0 tych0 merged commit 79c4f8d into tych0:cffi Jul 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants