search

mapado / datection

Detect and normalize temporal expressions
0 stars 0 forks source link

Rs fix parsing logic (last one) #32

Closed jdeniau closed 4 years ago

jdeniau commented 8 years ago

Pull request :twisted_rightwards_arrows: created by @RomainMapado on 2016-09-19 14:07 Last updated on 2017-02-13 08:51 Original Bitbucket pull request id: 32

Participants:

  • @badaz :heavy_check_mark:
  • @RomainMapado
  • @jdeniau (reviewer) :heavy_check_mark:
  • @jnieuviarts (reviewer)
  • @zwit (reviewer) :heavy_check_mark:
  • @dallegoet (reviewer) :heavy_check_mark:

Source: https://github.com/mapado/datection/commit/e461b0b5f694 on branch rs-fix-parsing-logic-2 Destination: https://github.com/mapado/datection/commit/d9d6ec2555ef on branch master

State: DECLINED

jdeniau commented 8 years ago

@RomainMapado commented on 2016-09-19 14:11

Location: line 1092 of datection/render.py

C'est pas "jojo" de passer un indice de liste en paramètre, mais la lib rrule de python m'a fait pas mal galérer (la version qu'on utilise a quelques bug de serialization et les versions ultérieures ont apparement des problèmes de compatibilité Python 2....)

jdeniau commented 8 years ago

@RomainMapado commented on 2016-09-19 14:14

Location: line 77 of datection/year_inheritance.py

Ce modulo donne: si l'event a lieu dans les 2 mois précédents, alors si il est terminé on considère qu'il était dans le passé

jdeniau commented 8 years ago

@jdeniau commented on 2016-09-21 07:42

Location: datection/render.py

C'est pas "jojo" de passer un indice de liste en paramètre, mais la lib rrule de python m'a fait pas mal galérer (la version qu'on utilise a quelques bug de serialization et les versions ultérieures ont apparement des problèmes de compatibilité Python 2....)

C'est peut-être le moment de passer à python 3 du coup ? :D adrien_fichet

jdeniau commented 8 years ago

@jdeniau commented on 2016-09-21 07:44

Location: line 1118 of datection/test/test_render.py

un peu bizarre le fait que excluded soit finalement une liste d'alteration non ?

Comment gères-tu les vrai exclusion ? ("Du vendredi 17 au mardi 21 janvier 2014 à 20 h 30, sauf le lundi 20")

jdeniau commented 8 years ago

@jdeniau commented on 2016-09-21 07:45

Location: datection/year_inheritance.py

Ce modulo donne: si l'event a lieu dans les 2 mois précédents, alors si il est terminé on considère qu'il était dans le passé

peut-être le mettre en commentaire dans le code ?

jdeniau commented 8 years ago

@jdeniau approved :heavy_check_mark: the pull request on 2016-09-21 07:46

jdeniau commented 8 years ago

@dallegoet approved :heavy_check_mark: the pull request on 2016-09-21 08:01

jdeniau commented 8 years ago

@zwit approved :heavy_check_mark: the pull request on 2016-09-23 08:16

jdeniau commented 8 years ago

@jnieuviarts commented on 2016-10-03 14:33

Evoqué avec Romain : ça ne me semble pas top d'ajouter une champ 'excluded_duration', assez complexe à appréhender pour un nouveau dev.

Je propose d'essayer de remplacer ces cas là par une inversion de logique : au lieu de mettre les horaires exclues, on ne stocke que les horaires inclues (même si on a parsé des horaires exclues). => un seul et unique moyen de stocker la même chose lu de 2 manières différentes

jdeniau commented 8 years ago

@badaz approved :heavy_check_mark: the pull request on 2016-10-27 09:14