-
Notifications
You must be signed in to change notification settings - Fork 19
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
Adaptation à la base JORF #54
base: master
Are you sure you want to change the base?
Conversation
With this patch legi.py can now read the JORF database, which has a structure very similar to LEGI. The main difference is the directory structure: articles are not in a subdirectory of the text but grouped together in a common directory. To avoid breaking back-compatibility the JORF must be put in a different SQLite database, and an additional db_meta parameter is added (called “base”, which can have values in ['JORF', 'LEGI']. Given the CID is not known in the path itself but only in the file, the file must be read a bit earlier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merci @Seb35 !
J'ai rajouté quelques changements pour aussi supporter KALI; cf https://github.com/Seb35/legi.py/pull/2/files. C'est en cours de travail mais il y a certaines parties non-spécifiques à KALI que tu pourrais inclure directement dans cette PR (notamment le .gitignore)
legi/tar2sqlite.py
Outdated
fpath = args.anomalies_dir + '/anomalies-' + last_update + '.txt' | ||
with open(fpath, 'w') as f: | ||
n_anomalies = detect_anomalies(db, f) | ||
print("logged", n_anomalies, "anomalies in", fpath) | ||
|
||
if not args.raw: | ||
if not args.raw and base == 'LEGI': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pareil, ça me semblerait plus propre de forcer l'utilisateur à utiliser l'option --raw
lorsqu'il utilise l'option --base JORF
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pourquoi pas, mais dans ce cas il faut documenter sur le README.md cette obligation pour ces bases.
Et de plus, similairement à l’option --base qui est stockée dans db_meta→base, ça pourrait être plus facile d’utilisation pour les mises à jour que de stocker cette valeur dans db_meta. Ainsi, legi.py appliquerait toujours les mêmes options lors des mises à jour. Mais ceci serait une autre PR.
legi/tar2sqlite.py
Outdated
@@ -467,6 +498,7 @@ def main(): | |||
p.add_argument('--pragma', action='append', default=[], | |||
help="Doc: https://www.sqlite.org/pragma.html | Example: journal_mode=WAL") | |||
p.add_argument('--raw', default=False, action='store_true') | |||
p.add_argument('--base') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
le défaut devrait être LEGI
non ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C’est ce que j’avais fait dans une première version, puis j’ai remplacé par les lignes 510-517 ci-après pour prioriser la valeur db_meta→base (qui elle-même vaut 'LEGI'
pour les bases existantes sans cette valeur pour rétro-compatibilité). Ainsi, dans le cas d’une mise à jour, l’utilisateur a le choix s’il veut forcer la vérification de la base :
- soit il ne met pas de paramètre --base et autorise legi.py à utiliser le paramètre db_meta→base,
- soit il met un paramètre --base pour obliger à ce que db_meta→base vaille exactement ce paramètre.
Si on met une valeur par défaut au paramètre --base, on ne peut plus avoir ce double comportement de vérification ou non et il faudrait toujours ajouter le paramètre --base avec la bone valeur. Ça complexifierait par exemple si on veut faire une boucle bash qui mettrait à jour plusieurs bases SQLite qui piocheraient dans un même dossier contenant les .tar.gz de toutes les bases.
Dans le cas d’une nouvelle base, la rédaction existante est l’équivalent d’un paramètre par défaut qui vaudrait 'LEGI'
.
legi/tar2sqlite.py
Outdated
@@ -532,13 +571,13 @@ def main(): | |||
print('last_update is now set to', last_update) | |||
|
|||
# Detect anomalies if requested | |||
if args.anomalies: | |||
if args.anomalies and base == 'LEGI': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ça me semblerait plus propre de lever une erreur si les deux options sont utilisées conjointement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Effectivement.
legi/tar2sqlite.py
Outdated
# https://github.com/Legilibre/legi.py/issues/23 | ||
try: | ||
unknown_folders[parts[2]] += 1 | ||
except KeyError: | ||
unknown_folders[parts[2]] = 1 | ||
continue | ||
dossier = parts[3] | ||
text_cid = parts[11] | ||
dossier = parts[3] if parts[0] == 'legi' else 'jorf' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
je crois que la notion de dossier n'existe pas pour les bases autres que LEGI. ça serait plus propre de stocker None
, non ?
Cela implique de changer le schema mais c'est mieux comme ça à mon avis. cf ma PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je m’étais posé la question, mais oui, effectivement, utiliser None
fait sens. J’avais mis 'jorf'
si jamais une base legi.py contient plusieurs bases (LEGI, JORF, …) mais il serait plus propre d’ajouter une colonne ou d’avoir recours à des tables séparées (par ex. legi_textes_versions) dans cette éventualité.
D’après cette page, il y a 3-4 autres bases avec des dossiers (CASS, INCA, JADE, voire ACCO même si le dossier "bureautique" est complètement hors schéma standard).
Je signale ici un problème qui se produit seulement sur le fichier JORF_20190201-213848.tar.gz qui contient semble-t-il des instructions de suppression et qui cause la levée d'une exception. Processing JORF_20190201-213848.tar.gz... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je n’ai pas encore regardé ta PR sur mon repo, je répond à ces remarques, et soumettrai prochainement un patch pour prendre en compte les 1, 3 et 4.
legi/tar2sqlite.py
Outdated
# https://github.com/Legilibre/legi.py/issues/23 | ||
try: | ||
unknown_folders[parts[2]] += 1 | ||
except KeyError: | ||
unknown_folders[parts[2]] = 1 | ||
continue | ||
dossier = parts[3] | ||
text_cid = parts[11] | ||
dossier = parts[3] if parts[0] == 'legi' else 'jorf' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je m’étais posé la question, mais oui, effectivement, utiliser None
fait sens. J’avais mis 'jorf'
si jamais une base legi.py contient plusieurs bases (LEGI, JORF, …) mais il serait plus propre d’ajouter une colonne ou d’avoir recours à des tables séparées (par ex. legi_textes_versions) dans cette éventualité.
D’après cette page, il y a 3-4 autres bases avec des dossiers (CASS, INCA, JADE, voire ACCO même si le dossier "bureautique" est complètement hors schéma standard).
legi/tar2sqlite.py
Outdated
@@ -467,6 +498,7 @@ def main(): | |||
p.add_argument('--pragma', action='append', default=[], | |||
help="Doc: https://www.sqlite.org/pragma.html | Example: journal_mode=WAL") | |||
p.add_argument('--raw', default=False, action='store_true') | |||
p.add_argument('--base') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C’est ce que j’avais fait dans une première version, puis j’ai remplacé par les lignes 510-517 ci-après pour prioriser la valeur db_meta→base (qui elle-même vaut 'LEGI'
pour les bases existantes sans cette valeur pour rétro-compatibilité). Ainsi, dans le cas d’une mise à jour, l’utilisateur a le choix s’il veut forcer la vérification de la base :
- soit il ne met pas de paramètre --base et autorise legi.py à utiliser le paramètre db_meta→base,
- soit il met un paramètre --base pour obliger à ce que db_meta→base vaille exactement ce paramètre.
Si on met une valeur par défaut au paramètre --base, on ne peut plus avoir ce double comportement de vérification ou non et il faudrait toujours ajouter le paramètre --base avec la bone valeur. Ça complexifierait par exemple si on veut faire une boucle bash qui mettrait à jour plusieurs bases SQLite qui piocheraient dans un même dossier contenant les .tar.gz de toutes les bases.
Dans le cas d’une nouvelle base, la rédaction existante est l’équivalent d’un paramètre par défaut qui vaudrait 'LEGI'
.
legi/tar2sqlite.py
Outdated
@@ -532,13 +571,13 @@ def main(): | |||
print('last_update is now set to', last_update) | |||
|
|||
# Detect anomalies if requested | |||
if args.anomalies: | |||
if args.anomalies and base == 'LEGI': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Effectivement.
legi/tar2sqlite.py
Outdated
fpath = args.anomalies_dir + '/anomalies-' + last_update + '.txt' | ||
with open(fpath, 'w') as f: | ||
n_anomalies = detect_anomalies(db, f) | ||
print("logged", n_anomalies, "anomalies in", fpath) | ||
|
||
if not args.raw: | ||
if not args.raw and base == 'LEGI': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pourquoi pas, mais dans ce cas il faut documenter sur le README.md cette obligation pour ces bases.
Et de plus, similairement à l’option --base qui est stockée dans db_meta→base, ça pourrait être plus facile d’utilisation pour les mises à jour que de stocker cette valeur dans db_meta. Ainsi, legi.py appliquerait toujours les mêmes options lors des mises à jour. Mais ceci serait une autre PR.
C’est plus subtil, car JORF_20190108-001815.tar.gz contient aussi une telle liste de suppression qui fonctionne. C’était le seul exemple sur la période 2018-11-29–2019-01-27, donc c’est un mécanisme qui n’a pas été beaucoup éprouvé. En l’espèce, la liste de suppression de JORF_20190201-213848.tar.gz est :
qui déclenche à juste titre l’exception. On ignore complètement cette vérification dans JORF spécifiquement (pour conserver la vérification dans LEGI) ? On autorise et ignore cette valeur |
|
Je suis en train de faire tourner pour tester cette PR, avec quelques détails supplémentaires que je pousserai (le problème du null dans liste_suppression.dat et des contraintes NOT NULL oubliées dans le schéma). |
Cette version fonctionne sur JORF sur la période 20181129-070000 à 20190209-005447. Les nouveaux commits changent ceci :
|
Super merci. |
J'ai constaté une chose étrange que je n'arrive pas à comprendre : il s'agit peut-être d'une différence entre la base JOFR construite par legy.py et la base JOFR publiée sur LegiFrance. J'ai observé cette différence par hasard sur un texte: JORFTEXT000037864346 Journal officiel du 28 décembre 2018 Item 17 : Arrêté du 27 décembre 2018 relatif à la prévention, à la réduction et à la limitation des nuisances lumineuses Si je cherche ce texte dans la base jofr.sqlite titrefull: Arrêté du 27 décembre 2018 relatif à la prévention, à la réduction et à la limitation des nuisances lumineuses Et si je cherche les articles associés: Or dans la version initiale du texte du mois de décembre, les 9 articles sont déjà présents : https://www.legifrance.gouv.fr/affichTexte.do;jsessionid=4A77CEBDCECB7D1A9D3DBF0B2573BF32.tplgfr28s_2?cidTexte=JORFTEXT000037864346&dateTexte=&oldAction=rechJO&categorieLien=id&idJO=JORFCONT000037864056 Je trouve étrange que les articles associés au texte aient un mtime de plus de 2 semaines de retard par rapport à la date supposée d'insertion de ces articles dans l'archive tgz. Par conséquent si je veux afficher le JO d'une date donnée, je ne peux me fier à mtime de la table article. Et le champ date_texte de la vue textes_versions_brutes_view est parfois renseigné mais souvent a la valeur 2999-01-01. Du coup je ne vois pas trop comment faire. Si quelqu'un à une explication ou une solution je suis preneur. |
Il y a plusieurs choses dans ce message. Je les prends une par une ci-après, mais globalement c’est une mauvaise compréhension de ce champ mtime.
Le champ mtime est la date de modification de l’enregistrement du texte, et il peut tout à fait y avoir des modifications "éditoriales" (ajouts de métadonnées, corrections de métadonnées, …) qui modifient cette date, qui peut alors être (largement) postérieure à la date de publication. J’ai également remarqué que cette date mtime varie de quelques secondes dans une livraison donnée, probablement car le processus d’export prend du temps et donc que ça enregistre des dates de modification légèrement différentes. En l’espèce, ce texte JORFTEXT000037864346 a été modifié au moins deux fois :
Je n’ai pas regardé les modifications apportées, mais tu peux le faire en décompressant ces deux fichiers et en faisant des diffs sur le texte et ses articles (fichiers dans texte/versions, texte/structs, section_ta et article). Je ne crois pas qu’il y ait d’outil pour ce faire puisque legi.py écrase les enregistrements. Ou disons, ça peut se faire avec Archéo Lex (je le fais sur LEGI) en enregistrant des branches Git tous les jours, mais il ne faut rater aucun jour et ça ne fait apparaitre que ce qui est réellement dans les données exportées par Archéo Lex (autrement dit le texte mais pas les métadonnées). Sur la remarque de l’affichage sur Légifrance, il est (très) probable que Légifrance ne s’appuie pas sur le champ mtime pour l’affichage du JORF mais plus probablement en affichant simplement tous les articles d’un texte paru au JORF (et s’appuie il s’appuie probablement sur le champ etat pour la base LEGI). Autrement dit, si tu cherches ce qui paraît au JORF, il faut plutôt que tu t’appuies sur le champ date_publi dans textes_versions. Sur les dates 2999-01-01, c’est la "norme" dans les bases DILA pour signifier "pas de date", soit parce que la date n’a pas été renseignée (dans le cas des vieux textes pas encore complètement saisis), soit parce que, dans les dates de fin, il n’y a pas de date de fin (la plupart des lois sont censées durer indéfiniment, du moins tant qu’elles ne sont pas abrogées). Je mentionne aussi pour information la date spéciale 2222-02-22 qui signifie "date d’entrée en vigueur future indéterminée" qui se trouve dans la base LEGI (il ne semble pas y en avoir dans JORF) lorsqu’une version consolidée en vigueur future est préparée, mais que la date d’entrée en vigueur est inconnue pour l’instant, généralement car cette date dépend d’un événement qui n’a pas encore été fixé (par exemple une date d’élection). Enfin, même si tu dois trouver l’information que tu cherches en utilisant la colonne |
Merci @Seb35 pour ces précisions j'apprends aussi des choses ! J'ai bien avancé sur le support de KALI dans ce projet @semiosys Si tu souhaites essayer en partant de ma branche KALI n'hésite pas, avec un peu de chance tout marchera sans aucune modification :) |
Merci beaucoup @Seb35 c'est beaucoup plus clair ! |
Hello @Changaco, est-ce qu'on peut aider pour merger cette PR ? Notre objectif c'était autant que possible de ne pas diverger mais de faire évoluer ce projet, tiens nous au courant si ça t'intéresse on est disponibles pour aider ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
schema.sql
est modifié mais il manque le changement correspondant dansmigrations.sql
- le contenu de la variable
base
devrait être normalisé en minuscules dès le début au lieu de mettre desbase.lower()
partout - les expressions comme
tag in ['ARTICLE', 'SECTION_TA']
sont à éviter, il vaut mieux utilisertag in {'ARTICLE', 'SECTION_TA'}
- je ne suis pas convaincu par le stockage d'une variable
base
dansdb_meta
, je préférerais un mécanisme plus générique qui prévoit dès maintenant la possibilité d'avoir plusieurs bases juridiques dans une seule base SQL
Bonsoir, Je rencontre un erreur dans le traitement des tarballs, ça bloque au 21 août dernier. J'ai vu que la branche principale legi avait été sensiblement retouchée récemment; peut-être que le branche JOFR n'a pas suivi cette évolution ? Si vous avez une idée je suis preneur ./cron/cron.sh
|
Hello @semiosys j'ai arrêté de bosser il y a un moment là dessus, je ne peux pas trop te dire. ça ressemble vaguement à un problème de version python 2/3 ou bien un argument par défaut incorrect mais je ne suis pas sûr. La version pour le code du travail numérique a bien divergé depuis, je t'encourage à aller voir : https://github.com/SocialGouv/dila2sql Bonne journée, |
@adipasquale Merci beaucoup Adrien. Je vais jeter un oeil du côté de dila2sql. En espérant que les modèles SQL n'aient pas trop changé par rapport à legilibre. |
Je suis désolé d’avoir laissé le temps filer pour cette PR. Je vais la reprendre prochainement. Par rapport à ta relecture, @Changaco, outre les 3 premiers points techniques que je corrigerai, je suis d’accord sur le 4e qui va changer assez fortement l’orientation de cette PR : prévoir un mécanisme pour stocker plusieurs bases DILA dans une même base SQLite. Pour le couple JORF+LEGI, ça sera d’autant plus utile puisque ça permettra des jointures entre les deux. Pour tout mettre dans une base SQLite, en théorie il n’y aurait pas besoin de modifier le schéma vu que les id de chaque base sont distincts par leurs préfixes (JORF, LEGI, CONS, etc.) et on pourrait remplir directement avec les autres bases. Mais dans l’hypothèse de cette solution, je pense qu’il faudrait ajouter une colonne 'base' dans toutes les tables avec un index dessus pour pouvoir interroger « juste une base DILA ». L’autre solution, radicalement différente, serait de dupliquer le schéma actuel avec un préfixe, on aurait par exemple jorf_textes_versions, jorf_articles. Je n’ai pas encore réfléchi si une solution est meilleure qu’une autre. @semiosys Je sais pas si tu as pu résoudre le blocage au 21 août 2019 : Changaco l’a corrigé dans #68. |
Bon, j’ai réfléchi et regardé quelques autres bases en détails (JORF, KALI), je penche plutôt pour faire des tables séparées avec un préfixe (jorf_textes_versions, jorf_articles) car même si 90 % des colonnes de JORF/KALI sont les mêmes que LEGI, il y a des subtilités qui seraient perdues au passage en SQL. Par exemple :
Aussi il serait réducteur d’utiliser la structure existante de LEGI pour les autres bases. Utiliser d’autres tables permettrait aussi d’avoir plus de granularité sur les index (par exemple on veut un index sur un champ de LEGI, mais pas forcément de JORF) et les éventuelles contraintes. Je propose donc d’utiliser des tables distinctes pour les différentes bases DILA. Cela demandera de l’expertise à chaque ajout de base, mais c’est sûrement mieux pour la qualité finale. Techniquement, cela évite tout problème de compatibilité sur ce point. Cela ferait diverger assez fortement de la première version de cette PR et donc du fork SocialGouv@dila2sql, mais je pense que c’est mieux dans le fond, et de toutes façons ce fork a très fortement divergé depuis un an. |
J’ai aussi resynchronisé en local cette PR sur la branche maîtresse, ça semble fonctionner mais je continue les changements. Juste, j’ai l’impression que c’est plus lent qu’auparavant (~5-10 items/s sur mon ordinateur alors que j’avais souvenir de quasiment un ordre de grandeur supérieur) ; c’est juste une impression venant de souvenirs peut-être brumeux d’il y a un an. |
À mon avis JORF et LEGI devraient probablement partager les mêmes tables, car LEGI contient de toute façon des éléments dont les identifiants commencent par Utiliser les mêmes tables ne signifie pas réduire les données au plus petit dénominateur commun. On peut soit ajouter une colonne pour chaque donnée supplémentaire, soit une seule colonne pour stocker toutes les données additionnelles en JSON.
SQLite gère les index partiels: https://sqlite.org/partialindex.html. |
Cette requête d’intégration s’inscrit dans le cadre de #33 et permet d’utiliser l’infrastructure existante de legi.py pour la base JORF.
La base JORF a quasiment la même structure que la base LEGI, à savoir des textes découpés en articles et en sections. La principale (et quasiment seule différence) est que tous les articles (resp. les sections, resp. les textes) sont dans le dossier 'article' (resp. 'section_ta', resp. 'texte'), au contraire de la base LEGI où toutes les données d’un texte sont dans un dossier LEGITEXT. Une différence minime de JORF est qu’il n’y a pas de grands dossiers comme "code_et_TNC_en_vigueur".
J’ai fait le choix dans cette implémentation de créer un fichier SQLite séparé pour la base JORF. C’est discutable, mais c’est une première proposition qui est fortement conservatrice. Une solution alternative serait de mettre toutes les bases dans un même fichier SQLite, peut-être avec des préfixes de tables (par ex. "legi_textes_versions") ; cela aurait l’avantage de permettre les requêtes SQL sur plusieurs bases, mais peut-être l’inconvénient que les fichiers SQLite deviennent énormes (legi.sqlite fait 4 Gio, jorf.sqlite 5 Gio, et il y a encore JADE qui est de l’ordre de LEGI, et KALI, CAPP, CASS de taille moyenne, le total d’une unique base ferait 20-25 Gio).
Pour différencier les bases SQLite, j’ai ajouté une métadonnée "base" dans db_meta qui peut valoir pour l’instant "LEGI" ou "JORF". Au lancement, il a une vérification de cette valeur pour interdire d’ajouter des données de LEGI dans une base JORF. S’il n’y a pas de telle métadonnée, la valeur utilisée est "LEGI" pour rétro-compatibilité.
À l’initialisation d’une nouvelle base SQLite, il peut être précisé un nouveau paramètre "--base JORF" pour télécharger ou créer une base JORF. Par défaut, une base LEGI est téléchargée ou créée. Lors de la mise à jour d’une base SQLite, ce paramètre peut être omis et il est alors lu la nature de la base SQLite ; ce paramètre peut toutefois être déclaré pour vérifier la nature de la base SQLite, et en cas de désaccord le programme ne va pas plus loin.