Fontbakery: Move on to Python 3

[felipesanches]

This issue was originally called "Unicode strings in python v2".

original description:

Review all strings with unicode in mind.

See: https://github.com/googlefonts/fontbakery/pull/1106

Python 3 port:

As of 2017 March 21, this is the place to track work on our Python 3 port as it should cleanup any bad handling of Unicode strings, byte strings, encoding/decoding operations etc. Python 3 is less forgiving on that, which is good since this is a key concern for FontBakery's code quality & correctness.

[felipesanches]

https://github.com/googlefonts/fontbakery/commit/965bb96b55cf201d28e48887a72807bfd4797e0e#commitcomment-19299810

[davelab6]

@felipesanches is there anything more to do here?

[felipesanches]

@davelab6 I'm looking at it again now.

[felipesanches]

Even though the issue is described as properly dealing with strings in python version 2, what really struck me as a more important question this morning was: "Why are we stuck on python version 2 at all? Why can't we move to python 3?". I could not easily remember exactly why we decided to stick to v2 in the past, so I investigated the issue and it seems the bad boy was Google Protobuffers, which used to support python v2-only. That's not the case anymore, so I decided it is time to seriously consider again moving on to v3.

I've been reviewing our usage of byte strings and unicode strings during the day and the bulk of the work is being done in this branch, which will likely be merged soon (once I'm done with the port and quality assurance):
https://github.com/felipesanches/fontbakery/tree/python3

[felipesanches]

I'm not exactly sure what's going on with this one in particular:

felipe@guarana:~/fb-dev (python3)*$ python3 ./fontbakery-fix-glyphs.py 
Traceback (most recent call last):
  File "./fontbakery-fix-glyphs.py", line 17, in <module>
    import glyphsLib
  File "<frozen importlib._bootstrap>", line 2237, in _find_and_load
  File "<frozen importlib._bootstrap>", line 2226, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 1191, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 1161, in _load_backward_compatible
  File "/usr/local/lib/python3.4/dist-packages/glyphsLib-1.0-py3.4.egg/glyphsLib/__init__.py", line 25, in <module>
  File "<frozen importlib._bootstrap>", line 2237, in _find_and_load
  File "<frozen importlib._bootstrap>", line 2222, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 2164, in _find_spec
  File "<frozen importlib._bootstrap>", line 1940, in find_spec
  File "<frozen importlib._bootstrap>", line 1916, in _get_spec
  File "<frozen importlib._bootstrap>", line 1897, in _legacy_get_spec
  File "<frozen importlib._bootstrap>", line 863, in spec_from_loader
  File "<frozen importlib._bootstrap>", line 904, in spec_from_file_location
  File "/usr/local/lib/python3.4/dist-packages/glyphsLib-1.0-py3.4.egg/glyphsLib/parser.py", line 119
    _unescape_re = re.compile(ur'(\\0[0-7]{2})|(\\U[0-9a-fA-F]{4,6})')
                                                                    ^
SyntaxError: invalid syntax

[davelab6]

I think we need Python 2 for legacy environments, and possible integration with font editors

[felipesanches]

Ouch!! Seriously ?! Or are you just guessing?

Mixed signals: https://github.com/googlefonts/fontbakery/issues/789

[felipesanches]

Fontbakery is pretty much stand-alone. I don't have any kind of integration with font editors in my radar. I only forsee command-line interface plus dockerized web dashboard.

[davelab6]

Could you work with the style of python 2 that is compatible with 3?

[davelab6]

Having said that, TruFont is the editor I would most want to integrate with, and that is python3 :)

[felipesanches]

@davelab6 That's surely a bit harder than simply moving on.
Do we really need that kind of additional constraint? The font editors integration did not sound compelling and "legacy environments need it" sounds a bit vague.

[felipesanches]

I'd like to move on fast not really caring much about python 2, if possible.

[felipesanches]

...unless people can point out solid use-cases that require backwards compatibility, of course.

[davelab6]

I'm fine to move to python3 asap :) Thanks for discussing :)

[felipesanches]

Alright! :-)

[davelab6]

How big an effort is this?

[felipesanches]

I can spent 15 minutes on it and then report back how long I think it will take.

[davelab6]

@felipesanches did we stay with python v2.7? If not, https://github.com/googlefonts/fontbakery/blob/b43272b8d42a69151efa5d6abcf6f7a9422dbf04/setup.py#L102 needs updating

[HinTak]

You can do this in every file as the first line:

from __future__ import unicode_literals

to see if it breaks. (this line is no-op in python 3, and basically for people who want to see if their code is ready to move from 2.7.x to python 3.x).

There are other such stuff in the __future__ module. It is a python 2.x module for people migrating to 3.x. By 2.x I think it means 2.7.x, or at most a late 2.6 - you do drop compatibility with older python 2.x if you do this. Argh, according to https://docs.python.org/2/reference/simple_stmts.html#future , uncode_literals is in python 2.6.

[felipesanches]

Full self-test coverage is a prerequisite before doing something risky like that.

I'm gradually increasing self-test coverage so that once I do things like this pytest will be my safety-net to avoid doing silly things and breaking things inadvertently.

[graphicore]

In most of the files we already use:

from __future__ import absolute_import, print_function, unicode_literals

For the actual test definitions in specifications/googlefonts.py we should add from __future__ import division and have a thorough review of our division cases. But, I'd be very happy to have python3 style explicit integer divisions //.

[madig]

Python 2 has a -3 switch that "warn[s] about Python 3.x incompatibilities that 2to3 cannot trivially fix". Augment all shebang lines with it? 😁

https://talkpython.fm/episodes/show/155/practical-steps-for-moving-to-python-3 has some more things.

[felipesanches]

Maybe...

What I do have in mind is that we should finish implementing all code-tests so that we can have them as a "safety net" when attempting a move to python 3.

[madig]

http://python-future.org/quickstart.html

[madig]

What tests are actually missing? I'd like to see a roadmap of some sort, we (especially I) want to get rid of Py2 already :grin:

[felipesanches]

the ones missing in googlefonts_test.py such as this one:
https://github.com/googlefonts/fontbakery/blob/fc9a4e68e5f002fe5df805b5fc6696b6bd1732cd/Lib/fontbakery/specifications/googlefonts_test.py#L612

[felipesanches]

Last stats that I gather was at version 0.3.2 and the code-tests covered 53% of the checks. As a code quality policy, recent additions of new checks have been made with corresponding code-checks. But there's not been a lot of new code tests for old checks. So I guess we still have roughly half of the checks needing proper code tests.

[madig]

Right now, running tox will test with 2.7. You can use tox -e py36 to test with 3.6 (provided you have it installed). Running futurize --stage1 Lib/**/*.py fixes many errors, but also points at (ab)use of lambdas like this: lambda x, (y, z): ..., when 3.x will only accept lambda x, y, z: .... This is all stuff that can probably be fixed in a long day, and actually I'd love to get cracking, but I'm a bit wary with the current PRs still in the queue. This work would touch most source files and constant rebasing and solving merge conflicts isn't fun. So... more tests first?

Getting rid of non-core-business scripts and dead test files would also help to bring coverage up, by the way ;)

[felipesanches]

@davelab6

This is pretty good! We now run our testsuite on Travis against both python 2.7 and python 3.6 on each pull request :-D

Next step is to increase out testsuite code-coverage so that we can be more and more certain that everything is rock-solid on both versions of the interpreter.

[felipesanches]

What tests are actually missing? I'd like to see a roadmap of some sort, we (especially I) want to get rid of Py2 already

@madig, I wrote boilerplate code for them all last week. You'll see on the test files that now we have tests called NOT_IMPLEMENTED_test_check_something. In their body I have also listed all code-paths that need testing so that we don't forget to cover them all. Here's an example:

https://github.com/googlefonts/fontbakery/blob/9784e47f393a059039a90dc4f83a39932d088755/tests/specifications/prep_test.py#L18-L27

[felipesanches]

I'll close this issue so that we do not convey the wrong idea that we might not yet support python 3.

We currently run automated tests on both python 2 and python 3 on travis. Code-test coverage is atroughly 60%, so increasing coverage is necessary, but we don't need an issue on the tracker for that.

[davelab6]

Sounds good!