etalab / transport-site

Rendre disponible, valoriser et améliorer les données transports
https://transport.data.gouv.fr
192 stars 30 forks source link

Jeu IRVE dynamique "valide mais pas valide" & notre usage de la spéc FrictionLess vs Validata #3895

Closed thbar closed 4 weeks ago

thbar commented 5 months ago

Je mets ça ici même si c'est pas un usage fréquent de faire ça, car ça a un impact sur mon design dans:

et je veux partager cette info entre collègues @etalab/transport-tech.

Constat

Le jeu https://www.data.gouv.fr/en/datasets/bornes-de-chargement-publiques-pour-voitures-electriques-du-plusieurs-operateurs/ n'est pas considéré invalide (pas de badge rouge) du côté de data gouv:

CleanShot 2024-04-24 at 11 42 33@2x

et "partiellement valide" du côté de validata:

https://validata.fr/table-schema?input=url&url=https%3A%2F%2Fstatic.data.gouv.fr%2Fresources%2Fbornes-de-chargement-publiques-pour-voitures-electriques-du-plusieurs-operateurs%2F20240424-090003%2Firve-dynamique.csv&header-case=on&schema_url=https%3A%2F%2Fschema.data.gouv.fr%2Fschemas%2Fetalab%2Fschema-irve-dynamique%2F2.3.1%2Fschema-dynamique.json

CleanShot 2024-04-24 at 11 43 45@2x

CleanShot 2024-04-24 at 11 43 56@2x

La spec elle-même est plus restrictive:

https://specs.frictionlessdata.io//table-schema/

It MUST contain a property fields. fields MUST be an array where each entry in the array is a field descriptor (as defined below). The order of elements in fields array SHOULD be the order of fields in the CSV file. The number of elements in fields array SHOULD be the same as the number of fields in the CSV file.

ce qui veut dire qu'un fichier, pour être complètement valide, doit avoir exactement les bonnes colonnes (toutes), et dans le bon ordre.

Impact

Dans le cadre d'une implémentation temps réel, il est nettement plus sain de s'assurer vu le débit, que les choses sont carrées à la base côté flux, si on en a l'opportunité, plutôt que de gérer de la permissivité dans le code.

C'est le choix que je fais actuellement dans #3839.

Cela permet de simplifier le code de façon conséquente, et c'est ce que je suis en train de faire.

Vu avec Stéphane

J'ai vu avec Stéphane pour qu'il demande au producteur de données d'ajouter les colonnes manquantes ; à défaut le jeu ne sera pas intégré à la consolidation (automatiquement car ça génère une erreur et le système de recovery fait que la donnée est droppée).

Ce ticket pourra servir d'appoint dans l'email.

AntoineAugusti commented 5 months ago

@thbar Tu souhaiterais obliger les producteurs à inclure les colonnes optionnelles (les colonnes etat_prise_*) ? Ça me parait compliqué de forcer les producteurs à produire ça car la documentation indique que celles-ci ne sont pas obligatoires.

Peut-on gérer la présence ou l'absence de ces colonnes de notre côté de manière à être un peu plus flexible avec les producteurs ? Peut-être dans un second temps quand on aura plus de producteurs de ces données. La consolidation BNLC a la même contrainte et est tolérante.

ptitfred commented 5 months ago

(newbie here:) La nuance est-elle "colonne vide" vs "colonne manquante" ?

AntoineAugusti commented 5 months ago

@ptitfred Thibaut faisait remarquer la différence (😢) entre TableSchema Specs et Validata.

TableSchema specs demande à ce que tous les champs soient présents dans la ressource (même quand required: false) et required indique si le champ peut contenir des valeurs null ou vides.

Pour la communauté open data FR ces données sont produites de manière régulière par des utilisateurs non experts et il me semble que le choix a été fait de tolérer que ces champs soient absents du CSV quand required: false pour faciliter la production quand un producteur n'est pas en mesure de renseigner les champs optionnels. Je ne retrouve plus de trace d'issues où c'est abordé en quelques minutes.

Validata tolère ça et indique un warning alors que TableSchema est plus strict. Ça crée une difficulté à gérer la présence ou l'absence quand on réutilise les données…

Un peu de code qui indique les champs optionnels à ajouter Le check custom forçant la présence des colonnes requises

thbar commented 5 months ago

Merci pour les commentaires - je répondrai plus tard avec + de détails, car demander cela n'est pas très problématique en pratique sur le cas dont on parle, et apporte pas mal d'avantages quand on traite du temps réel. À suivre !

thbar commented 5 months ago

Je réponds, on pourra évoquer ça au point dév oralement ça sera pas plus mal et plus convivial je pense, vu les pavés de texte que ça fait 😃

Etat de la documentation

La documentation (https://schema.data.gouv.fr/etalab/schema-irve-dynamique/2.3.1/documentation.html) est ambiguë en effet, et ce point devra être pris en compte (même pour d’autres schémas) car du coup cela ne reflète pas correctement ce qui est spécifié au niveau du schéma lui-même.

Le schéma IRVE suit pour son formalisme la spécification TableSchema Frictionless:

https://github.com/etalab/schema-irve/blob/master/dynamique/schema-dynamique.json#L2

Cette spécification stipule (je peux écrire stipuler quand ils utilisent MUST !) de façon non ambiguë ici:

https://datapackage.org/specifications/table-schema/#fieldsmatch

The data source MUST have exactly the same fields as defined in the fields array. Fields MUST be mapped by their order.

Et également (désolé pour la redite):

https://specs.frictionlessdata.io//table-schema/

It MUST contain a property fields. fields MUST be an array where each entry in the array is a field descriptor (as defined below). The order of elements in fields array SHOULD be the order of fields in the CSV file. The number of elements in fields array SHOULD be the same as the number of fields in the CSV file.

Donc la documentation (auto-générée) ne reflète simplement pas la spécification correctement (et il faudra voir comment adresser ce point dans un sens ou dans un autre).

Charge à nous de mettre la documentation auto-générée de schema.data.gouv.fr en synchro avec le schéma lui-même (ou l’inverse selon nos conclusions).

-> Sur ce point, j’ouvrirai un ticket car ça peut créer des ambiguités

Conséquences pratiques (validation)

Vu que le schéma s'appuie sur Frictionless, et si on passe un fichier sans ces colonnes optionnelles au validateur Frictionless (qui est le validateur canonique du système sur lequel le schéma s'appuie pour définir sa propre structure), un réutilisateur ou un validateur observera des erreurs:

❯ frictionless validate \
--schema https://schema.data.gouv.fr/schemas/etalab/schema-irve-dynamique/latest/schema-dynamique.json \
https://www.data.gouv.fr/fr/datasets/r/b20d2793-db42-4d6d-a0b4-e94bf5ee4279 \
--json --format csv | head -10
{
  "valid": false,
  "stats": {
    "tasks": 1,
    "errors": 1000,
    "warnings": 1,
    "seconds": 0.596
  },
  "warnings": [],
  "errors": [],

Ceci obligerait quelqu'un qui veut vérifier si un flux est valide par les moyens proposés par exactement le framework utilisé pour définir le schéma, à filtrer & post-processer les erreurs de ce type (sauf à revoir la copie plus haut, et rendre via la propriété fieldsmatch les colonnes réellement optionnelles), ce qui est fragile dans le temps.

Validata tolère ça et indique un warning alors que TableSchema est plus strict. Ça crée une difficulté à gérer la présence ou l'absence quand on réutilise les données…

Validata fait un reporting plus souple certes (et @pierrecamilleri pourra apporter des précisions sur ce point), mais il est vraiment souhaitable que le validateur de référence de l'outil qu'on a utilisé pour définir le schéma, considère la donnée valide lui-même.

À titre personnel je trouverais sain que Validata et Frictionless convergent autant que possible (mais je comprends bien les compromis faits), pour éviter justement ce genre d’ambiguités !

Conséquences techniques (temps réel)

Peut-on gérer la présence ou l'absence de ces colonnes de notre côté de manière à être un peu plus flexible avec les producteurs ? Peut-être dans un second temps quand on aura plus de producteurs de ces données. La consolidation BNLC a la même contrainte et est tolérante.

Quand on fait une ouverture de fichiers statiques (comme la majorité des cas jusque là, et comme la BLNC), on a moins de contraintes qu’en temps réel.

En temps réel, les contraintes ne sont pas les mêmes (optimisations différentes).

La spécification par défaut de Frictionless prend toute sa valeur pour du temps réel: être sûr que les champs sont les bons, dans le bon ordre, permet de se poser moins de questions quand il y a du débit.

Pour donner un ordre de grandeur, si on a 60k lignes à agréger, et qu’on fait ce travail toutes les minutes (ou même un peu moins), on va traiter 72 millions de lignes de CSV par jour.

Si on peut éviter de faire trop d’inférence ou de souplesse là dedans, la maintenance et le débuggage seront fortement simplifiés (et on gagnera du temps).

Ce point s’applique également à toute personne consommant la donnée !

Maturation de l’écosystème

La consolidation BNLC a la même contrainte et est tolérante.

La tolérance est une bonne chose au début de la maturation de l’écosystème. Par contre elle n’arrange personne quand l’écosystème murit: il est préférable de remonter l’homogénéité upstream, pour éviter que chaque consommateur “non consolidé” doive traiter ce même genre de choses.

Si on observe au fil du temps, de nombreux sujets évoluent de “tolérance” à “resserrage de vis”, justement car la tolérance a un coût (et c’est pour ça qu’on transforme des warnings en erreurs avec le temps):

Donc sur ce point, la tolérance est bien au début, et au fur et à mesure qu’un écosystème murît, on a intérêt plutôt à resserrer les vis car sinon ça coûte cher un peu partout.

(Évidemment là on est sur un point un peu plus large et philosophique aussi, si on revient au fichier dont on parle, on parle de 4 colonnes vides à ajouter !)

Incitation à la production de la donnée (lobbying)

Je peux ajouter également qu’étant l’auteur du schéma dynamique, mon idée sur ces colonnes étaient de toute façon qu’elles soient optionnelles de façon temporaire uniquement, et d’encourager plutôt les producteurs à les inclure, que pas.

Envoyer le message que la colonne elle-même peut ne pas apparaître du tout, n’incite pas le producteur à aller la remplir à un moment tout court.

Donc à titre personnel je préfère forcer un peu la main et demander la production régulièrement, que d’invisibiliser cette possibilité en laissant supprimer ces 4 colonnes précises.

On peut par ailleurs utiliser le fait que l’inclusion dans le concentrateur soit conditionnée à la validité complète de la donnée comme quelque chose de vertueux et donnant / donnant.

Nombre de producteurs concernés (1 seul pour le moment)

Si on revient au schéma précisément évoqué, il s’agit de données produites en dynamique, pour des points de charge électrique.

Pour la communauté open data FR ces données sont produites de manière régulière par des utilisateurs non experts

Ce point n’est pas exact concernant les IRVE dynamiques.

Ces données ne seront pas produites par des entités non techniques, donc je ne pense pas qu’imposer ça soit un gros frein pour les entités considérées.

On a pour l’instant de 3 flux, dont 2 sont déjà conformes d’office sans leur avoir rien demandé.

J’ai pris le parti de demander au dernier producteur inclus dans la configuration de départ, si cette modification était faisable.

Impact TIRUERT (subvention)

On verra ce qu'en pense @jmaupetit, mais je crois que ces informations optionnelles aujourd'hui intéresseront peut-être Qualicharge de façon plus appuyée (ils utilisent ce schéma également), aussi il sera bon d'en parler avec eux pour voir dans quelle direction on souhaite amener le schéma.

jmaupetit commented 5 months ago

Dans une approche bad cop, l'idéal pour nous serait d'être pédantiques :wink: ! Donc exiger de remplir tous les champs serait idéal.

thbar commented 5 months ago

J'ai demandé + d'infos au fournisseur, je vous tiendrai au courant !

AntoineAugusti commented 5 months ago

@thbar C'est envisageable de changer à required: true pour toutes les colonnes sur le schéma ?

thbar commented 5 months ago

@AntoineAugusti je regarderai ; je pense qu'on risque de perdre GIREVE mais à confirmer. Et concernant le jeu d'Eco-Movement il serait d'office invalide en l'état, j'attends leur retour.

pierrecamilleri commented 5 months ago

Hello, permettez-moi d'ajouter quelques informations contextuelles.

Pour la communauté open data FR ces données sont produites de manière régulière par des utilisateurs non experts et il me semble que le choix a été fait de tolérer que ces champs soient absents du CSV quand required: false pour faciliter la production quand un producteur n'est pas en mesure de renseigner les champs optionnels. Je ne retrouve plus de trace d'issues où c'est abordé en quelques minutes.

Je confirme, @johanricher a fait un peu d'archéologie pour nous ;) (merci :pray:)

L'option frictionless qui est utilisée dans validata est Schema Sync (option --schema-sync avec frictionless cli)

À noter, que la v2 de la spécification Table Schema va apporter du nouveau sur cette thématique avec la propriété fieldsMatch qui indiquera directement, schéma par schéma, le comportement souhaité avec les colonnes manquantes, les colonnes surnuméraires, et l'ordre des colonnes. Cela (à mon sens) déprécie l'option "Schema sync", et une fois la spec v2 adoptée, Validata devra évidemment se plier à la directive du schéma.

À titre personnel, je trouve nettement plus pertinent de préciser le comportement attendu au niveau du schéma qu'au moment de la validation , donc je salue cette évolution. Reste à choisir ce que vous souhaitez comme comportement, ce pour quoi j'ai l'impression que vous avez déjà avancé tous les arguments pour et contre dans ce fil !

thbar commented 5 months ago

Merci @pierrecamilleri et @johanricher pour le coup d'archéologie et pour le retour.

Et poke @Pierlou que ça va intéresser 😄

Reste à choisir ce que vous souhaitez comme comportement, ce pour quoi j'ai l'impression que vous avez déjà avancé tous les arguments pour et contre dans ce fil !

Au delà du cas précis levé (1 producteur avec 1 flux et ils sont en train d'évaluer), c'est vrai qu'homogénéiser tout ça pour le bien de tout le monde (en dehors de ce cas précis) sera un + !

thbar commented 4 months ago

La demande a été faite au producteur il y a quelques jours, qui ne voit pas de souci à ajouter les colonnes et nous tiendra au courant.

thbar commented 4 weeks ago

Après avoir fait une visio avec le producteur récemment, ils n'ont vu aucun problème à ajouter les 4 headers des colonnes qu'ils ne remplissent pas actuellement, ont fait la modification, et m'ont informé que c'était en production (j'ai pu aller vérifier l'état de la donnée, c'est OK).

Le problème est donc réglé concernant Eco Movement, je vais pouvoir clôturer ce ticket !

De ce fait:

On est bon, ça nous fait donc un fichier de + dans le jeu, en attendant de toute façon très prochainement (début octobre) une agrégation intermédiaire fournie par QualiCharge (échanges en cours avec @jmaupetit !)

Frictionless à présent:

❯ frictionless validate b20d2793-db42-4d6d-a0b4-e94bf5ee4279 --schema https://schema.data.gouv.fr/schemas/etalab/schema-irve-dynamique/latest/schema-dynamique.json --json --format csv
{
  "valid": true,
  "stats": {
    "tasks": 1,
    "errors": 0,
    "warnings": 0,
    "seconds": 0.455
  },
  "warnings": [],
  "errors": [],
  "tasks": [
    {
      "name": "b20d2793-db42-4d6d-a0b4-e94bf5ee4279",
      "type": "table",
      "valid": true,
      "place": "b20d2793-db42-4d6d-a0b4-e94bf5ee4279",
      "labels": [
        "id_pdc_itinerance",
        "etat_pdc",
        "occupation_pdc",
        "horodatage",
        "etat_prise_type_2",
        "etat_prise_type_combo_ccs",
        "etat_prise_type_chademo",
        "etat_prise_type_ef"
      ],
      "stats": {
        "errors": 0,
        "warnings": 0,
        "seconds": 0.455,
        "md5": "39a1c2ab933390df37aa6e9169fc306b",
        "sha256": "dc400e1c70dbb888d662968e0f8d3a2deec54ca62840024f41fa3bd9bfa61762",
        "bytes": 749591,
        "fields": 8,
        "rows": 13223
      },
      "warnings": [],
      "errors": []
    }
  ]
}

Frictionless avec une colonne qui manque (même si la donnée elle même est optionnelle):

❯ frictionless validate b20d2793-db42-4d6d-a0b4-e94bf5ee4279 --schema https://schema.data.gouv.fr/schemas/etalab/schema-irve-dynamique/latest/schema-dynamique.json --json --format csv
{
  "valid": false,
  "stats": {
    "tasks": 1,
    "errors": 1,
    "warnings": 0,
    "seconds": 0.462
  },
  "warnings": [],
  "errors": [],
  "tasks": [
    {
      "name": "b20d2793-db42-4d6d-a0b4-e94bf5ee4279",
      "type": "table",
      "valid": false,
      "place": "b20d2793-db42-4d6d-a0b4-e94bf5ee4279",
      "labels": [
        "id_pdc_itinerance",
        "etat_pdc",
        "occupation_pdc",
        "horodatage",
        "etat_prise_type_2",
        "etat_prise_type_combo_ccs",
        "etat_prise_type_chademo"
      ],
      "stats": {
        "errors": 1,
        "warnings": 0,
        "seconds": 0.462,
        "md5": "76af7320c004499aeb7849a2a54d2cfc",
        "sha256": "892bedd1b99e889500b72ae06713eed7e5e9f7e4b44a96a191ba30248ff150ee",
        "bytes": 749572,
        "fields": 8,
        "rows": 13223
      },
      "warnings": [],
      "errors": [
        {
          "type": "missing-label",
          "title": "Missing Label",
          "description": "Based on the schema there should be a label that is missing in the data's header.",
          "message": "There is a missing label in the header's field \"etat_prise_type_ef\" at position \"8\"",
          "tags": [
            "#table",
            "#header",
            "#label"
          ],
          "note": "",
          "labels": [
            "id_pdc_itinerance",
            "etat_pdc",
            "occupation_pdc",
            "horodatage",
            "etat_prise_type_2",
            "etat_prise_type_combo_ccs",
            "etat_prise_type_chademo"
          ],
          "rowNumbers": [
            1
          ],
          "label": "",
          "fieldName": "etat_prise_type_ef",
          "fieldNumber": 8
        }
      ]
    }
  ]
}
thbar commented 3 weeks ago

Et comme vu avec @jmaupetit semaine dernière, Qualicharge (qui va devenir un fournisseur IRVE central pour le PAN à partir du 1er octobre, en statique autant qu'en dynamique) va avoir des exigences un peu plus fortes que "juste le schéma" pour des raisons variées (qualité, possibilité d'appuyer grâce à subventions etc), donc on va dans le même sens là dessus, plutôt à appliquer déjà le schéma au format "pédantique" comme dit Julien, mais même à aller un peu plus loin.

En conclusion : ne pas hésiter à resserrer les boulons petit à petit dans l'intérêt de l'information voyageur et de la simplicité de notre propre code, aucun souci ici au final.