search

mapado / datection

Detect and normalize temporal expressions
0 stars 0 forks source link

Make Datection Python 3 compatible #75

Closed jdeniau closed 4 years ago

jdeniau commented 6 years ago

Pull request :twisted_rightwards_arrows: created by @jnieuviarts on 2017-12-10 10:30 Last updated on 2017-12-12 09:21 Original Bitbucket pull request id: 75

Participants:

  • bitbucket user devops
  • @RomainMapado (reviewer) :heavy_check_mark:
  • bitbucket user adrien_fichet (reviewer)
  • @thomasdiluccio (reviewer)
  • @jdeniau (reviewer) :heavy_check_mark:
  • @jnieuviarts
  • @dallegoet (reviewer) :heavy_check_mark:

Source: https://github.com/mapado/datection/commit/9b9072a2d3bc on branch jn_python3_compatible Destination: https://github.com/mapado/datection/commit/c6b54437b71d on branch master Merge commit: https://github.com/mapado/datection/commit/1b51c7be7b0d

State: MERGED

(les tests ne passent pas sur jenkins mais ça semble un pb spécifique jenkins que j'essaye de fixer avec Adrien. Ils passent sinon).

Principales modifications (et enseignements)

La "cheat sheet" de bascule python 2 vers python 3 est très pratique.

Le package "futurize" permet de dégrossir le boulôt, il se contente toutefois des correctifs évidents.

#!python
pip install future
futurize --stage1 mypackage/**/*.py
futurize --stage2 mypackage/**/*.py

Pour faciliter la basculer : créer 2 virtualenv pour le package mypackage_PY2 et mypackage_PY3.

#!bash

mkvirtualenv --python=/usr/bin/python2 my_package_PY2
#!bash

mkvirtualenv --python=/usr/bin/python3 my_package_PY3

A chaque itération / modification, vérifier en changeant le virtual env que les tests passent.

Les grands changements rencontrés :

les dictionnaires ne sont plus ordonnés sur la clé par défaut

Il faut ajouter des "sorted" à de multiples endroits pour forcer le tri si on veut des tests reproductibles.

Utiliser (compatible python2 et python3)

#!python

[v for k, v in sorted(out.items())]

à la place de (compatible uniquement python2)

#!python

out.values()

les dictionnaires ne sont plus itérables

Il faut les convertir en list

#!python

for item in list(my_dict)

xrange n'existe plus

utiliser range tout simplement

la division est par défaut en float

Alors qu'en python 2, elle est en int. Pour conserver l'ancien fonctionnement, importer le module old_div

#!python

from past.utils import old_div
zero = old_div(1, 2))

.decode('utf-8') n'est plus utile

Il plante en python 3. Il faut donc ajouter une condition spécifique "if six.PY2:......" pour ajouter la méthode en fonction de la version

Comme utf-8 est le défaut en python3, la fonction unicode n'est pas utile

Utiliser (compatible python2 et python3)

#!python

from builtins import str
str(my_string)

à la place de (compatible uniquement python2)

#!python

from builtins import str
unicode(my_string

assertItemsEqual n'existe plus en python3

Utiliser (compatible python2 et python3)

#!python

six.assertCountEqual(self, new_details, res_details)

à la place de (compatible uniquement python2)

#!python

self.assertItemsEqual(new_details, res_details)

le module, plutôt cool, freezegun permet de forcer datetime.today dans un test

#!python

freeze_time("2012-01-01")
class TestDatetimeList(unittest.TestCase):

plutôt que d'utiliser un mock de datetime.today() que je n'ai pas réussi à faire fonctionner en python 3

#!python

class TestDatetimeList(CurrentDayMock):

print "toto" n'est plus possible, utiliser print("toto")

il est nécessaire d'importer 

#!python

from __future__ import print_function

certains objects ne sont plus hashable par défaut

Il faut donc soit créer une méthode de hashage, soit utilise id(object) qui génère un id unique. Par ailleurs, il est obligatoire d'avoir une propriété hash si on définit une propriété eq

#!python

def __hash__(self):
    return hash((self.date_interval, self.time_interval, (str(d) for d in self.weekdays)))
jdeniau commented 6 years ago

@RomainMapado commented on 2017-12-11 08:20

"assertCountEqual" est tellement mal nommé :(

jdeniau commented 6 years ago

@jnieuviarts commented on 2017-12-11 10:41

"assertCountEqual" est tellement mal nommé :(

Oui, j'ai vu des débats sur le sujet quand j'ai cherché à solutionner le pb ;-)

jdeniau commented 6 years ago

Bitbucket user devops commented on 2017-12-11 10:42

TTP build flag [bid: #jenkins-57589563436264420a1f5921b291cad3]

jdeniau commented 6 years ago

@thomasdiluccio commented on 2017-12-11 10:55

Outdated location: line 115 of datection/rendering/utils.py

tu peux supprimer cela

jdeniau commented 6 years ago

@thomasdiluccio commented on 2017-12-11 10:58

Outdated location: line 116 of datection/test/test_timepoint.py

Tu es certain pour le "0o3" ?

jdeniau commented 6 years ago

@thomasdiluccio commented on 2017-12-11 10:59

Outdated location: line 314 of datection/test/test_timepoint.py

On doit pouvoir supprimer cet test intégralement commenté

jdeniau commented 6 years ago

@thomasdiluccio commented on 2017-12-11 10:59

Location: line 372 of datection/test/test_timepoint.py

pareil ?

jdeniau commented 6 years ago

@thomasdiluccio commented on 2017-12-11 11:05

Outdated location: line 19 of datection/utils.py

Je ne comprends pas les 0o1

jdeniau commented 6 years ago

@jdeniau commented on 2017-12-11 13:37

Rho trop bien :)

Pour les tests sur python 2 et 3, dommage de ne pas avoir un truc "à la travis". Peut-être que les pipeline jenkins font le job :man_shrugging:

jdeniau commented 6 years ago

@jdeniau approved :heavy_check_mark: the pull request on 2017-12-11 13:39

jdeniau commented 6 years ago

@dallegoet approved :heavy_check_mark: the pull request on 2017-12-11 15:26

jdeniau commented 6 years ago

@RomainMapado commented on 2017-12-11 15:33

Outdated location: line 184 of datection/rendering/long.py

On peut vraiment pas garder values comme iterateur ici? (c'est dommage ce k inutile)

jdeniau commented 6 years ago

@RomainMapado commented on 2017-12-11 15:37

Location: line 1009 of datection/timepoint.py

je crois même qu'on peut virer les [] ici (à vérifier)

jdeniau commented 6 years ago

@RomainMapado approved :heavy_check_mark: the pull request on 2017-12-11 15:38

jdeniau commented 6 years ago

@jnieuviarts commented on 2017-12-11 15:42

Location: datection/rendering/long.py

On peut vraiment pas garder values comme iterateur ici? (c'est dommage ce k inutile)

Je n'ai pas trouvé mais sorted en python 3 renvoie un tableau de tuple [(key, value), (key, value)....] On peut éventuellement mettre _ il me semble pour indiquer que ça n'est pas utilisé ensuite

jdeniau commented 6 years ago

@jnieuviarts commented on 2017-12-11 21:59

Rho trop bien :)

Pour les tests sur python 2 et 3, dommage de ne pas avoir un truc "à la travis". Peut-être que les pipeline jenkins font le job :man_shrugging:

@jdeniau Si tu as une idée, en gros, il "suffirait" de créer 2 virtualenv, 1 en python 2 et un en python 3 et lancer 2 fois les tests. Tu saurais comment faire ça ?

jdeniau commented 6 years ago

@jnieuviarts commented on 2017-12-11 22:04

Location: datection/utils.py

Je ne comprends pas les 0o1

@thomasdiluccio En gros dans python 3, il n'est plus possible d'avoir des "leading zeros" dans un int. Sauf avec cette astuce. C'est la lib de convertion qui avait fait ça en auto. Toutefois, il suffit de mettre sans leading zero et ça fonctionne. Je vais le corriger.

jdeniau commented 6 years ago

@jnieuviarts commented on 2017-12-12 08:56

Location: datection/timepoint.py

je crois même qu'on peut virer les [] ici (à vérifier)

@RomainMapado Bien vu, ça fonctione