Development #38067
Revoir complètement la génération des tableaux et la gestion des valeurs NULL dues aux OUTER JOIN
0%
Description
Pour chaque type de dimension on peut définir un label d'absence.
- voir tous les éléments des dimensions dans tous les cas
- avoir la valeur Aucun(e) dans tous les cas où une dimension peut-être absente (ici c'est détecté à la volée en fonction des données)
J'en ai profité pour remettre à plat la façon dont les résultats des requêtes sont produits (on conserve les objets Dimension et Measure tout du long désormais plutôt que de passer par des dicos mal définis).
Fichiers
Demandes liées
Révisions associées
tests: use tabulate to compare tables (#38067)
Comparing strings with assert gives better diffs thant comparing list of
lists when using the pytest's `-vv` option.
tests: reformat json files for easier update (#38067)
Add script format_json_fixtures.py to canonicalize every JSON files in
fixtures to simplify management of changes in visualization results.
tests: add option to update tables during tests (#38067)
If table contents change, we can measure the change and eventually
accept by copying new_table.json to tables.json. To activate it, use
--bijoe-store-table .
utils: cache get_warehouses() result based on schema files paths (#38067)
engine: cache EngineDimension.members (#38067)
engine: do not hardcode cube's json field name (#38067)
engine: cache proxy descriptor result on engine's cubes (#38067)
engine: hide NULL values of json dimensions in inline join table (#38067)
engine: use LEFT OUTER JOIN to join with inline json join table (#38067)
schemas: add an absent_label property to Dimension (#38067)
Also force lazy strings to unicode in to_json().
visualization/utils: remove dead import (#38067)
overhaul of query to table transformation (#38067)
- during query obtain dimension id and label if a different projection
is defined - use object to materialize query results : Cells, DimensionCell,
MeasureCell - handle stringification in the *Cell classes
- never ignore NULL dimension's values (it's detected in
Visualization.data() and added to the list of dimension members) - sum of columns is only computed if there are more than one column
- sum of rows is only computed if there are more than one row
- full sum is only computed if there are more thant one column and more
than one row - 1 dimension table are computed in the same maner as 2 dimensions
tables, no more discrepancies - JSON web-service now use the same base methods table_2d() and table_1d()
as the native rendering in bijoe - EngineDimension.members is specialized for bool dimensions (as it's
always True/False)
engine: propagate filters to dimension's members enumeration (#38067)
settings: improve tests runtime by turning DEBUG off (#38067)
Historique
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
- Fichier 0001-schemas-add-an-absent_label-property-to-Dimension-38.patch 0001-schemas-add-an-absent_label-property-to-Dimension-38.patch ajouté
- Patch proposed changé de Non à Oui
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
- Sujet changé de Définir une valeur d'absence pour une dimension à Définir un label d'absence pour une dimension
- Description mis à jour (diff)
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
- Fichier 0004-tests-keep-new-table-contents-during-tests-38067.patch 0004-tests-keep-new-table-contents-during-tests-38067.patch ajouté
- Fichier 0003-tests-reformat-json-files-for-easier-update-38067.patch 0003-tests-reformat-json-files-for-easier-update-38067.patch ajouté
- Fichier 0008-engine-cache-proxy-descriptor-result-on-engine-s-cub.patch 0008-engine-cache-proxy-descriptor-result-on-engine-s-cub.patch ajouté
- Fichier 0006-engine-cache-EngineDimension.members-38067.patch 0006-engine-cache-EngineDimension.members-38067.patch ajouté
- Fichier 0005-utils-cache-get_warehouses-result-based-on-schema-fi.patch 0005-utils-cache-get_warehouses-result-based-on-schema-fi.patch ajouté
- Fichier 0009-engine-hide-NULL-values-of-json-dimensions-in-inline.patch 0009-engine-hide-NULL-values-of-json-dimensions-in-inline.patch ajouté
- Fichier 0010-engine-use-LEFT-OUTER-JOIN-to-join-with-inline-json-.patch 0010-engine-use-LEFT-OUTER-JOIN-to-join-with-inline-json-.patch ajouté
- Fichier 0011-schemas-add-an-absent_label-property-to-Dimension-38.patch 0011-schemas-add-an-absent_label-property-to-Dimension-38.patch ajouté
- Fichier 0012-visualization-utils-remove-dead-import-38067.patch 0012-visualization-utils-remove-dead-import-38067.patch ajouté
- Fichier 0007-engine-do-not-hardcode-cube-s-json-field-name-38067.patch 0007-engine-do-not-hardcode-cube-s-json-field-name-38067.patch ajouté
- Fichier 0001-tests-load-schema2-fixture-only-one-time-38067.patch 0001-tests-load-schema2-fixture-only-one-time-38067.patch ajouté
- Fichier 0013-overhaul-of-query-to-table-transformation-38067.patch 0013-overhaul-of-query-to-table-transformation-38067.patch ajouté
- Fichier 0002-tests-use-tabulate-to-compare-tables-38067.patch 0002-tests-use-tabulate-to-compare-tables-38067.patch ajouté
- Statut changé de Nouveau à Solution proposée
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
- Sujet changé de Définir un label d'absence pour une dimension à Revoir complètement la génération des tableaux et des valeurs NULL due aux OUTER JOIN
- Description mis à jour (diff)
Mis à jour par Frédéric Péters il y a plus de 4 ans
format_json_fixtures.py, dans le json.dump, j'ajouterais separators=(',', ': '), pour ne pas avoir d'espace en fin de ligne. (ou je forcerais à exécuter le script avec Python >= 3.4, où c'est devenu le paramétrage par défaut.
return unicode(self.label); ce serait bien de prendre en compte Python 3 dans le nouveau code.
File "/home/fred/src/eo/bijoe/bijoe/visualization/views.py", line 311, in get data = cell_value(visualization.data()[0][0]) File "/home/fred/src/eo/bijoe/bijoe/visualization/views.py", line 281, in cell_value if cell.measure.type == 'duration' and cell.value is not None: AttributeError: 'list' object has no attribute 'measure'
Sur une visualisation avec juste "nombre de demandes", pas de regroupements/répétitions, rien. (cell vaut [])
Sur le fond, je ne connais pas assez pour relire, mais ça m'irait d'avoir une petite description de ce qui a été entrepris dans ces patchs. (la description revue contient déjà un peu plus d'infos).
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
- Fichier 0004-tests-keep-new-table-contents-during-tests-38067.patch 0004-tests-keep-new-table-contents-during-tests-38067.patch ajouté
- Fichier 0003-tests-reformat-json-files-for-easier-update-38067.patch 0003-tests-reformat-json-files-for-easier-update-38067.patch ajouté
- Fichier 0008-engine-cache-proxy-descriptor-result-on-engine-s-cub.patch 0008-engine-cache-proxy-descriptor-result-on-engine-s-cub.patch ajouté
- Fichier 0006-engine-cache-EngineDimension.members-38067.patch 0006-engine-cache-EngineDimension.members-38067.patch ajouté
- Fichier 0005-utils-cache-get_warehouses-result-based-on-schema-fi.patch 0005-utils-cache-get_warehouses-result-based-on-schema-fi.patch ajouté
- Fichier 0009-engine-hide-NULL-values-of-json-dimensions-in-inline.patch 0009-engine-hide-NULL-values-of-json-dimensions-in-inline.patch ajouté
- Fichier 0010-engine-use-LEFT-OUTER-JOIN-to-join-with-inline-json-.patch 0010-engine-use-LEFT-OUTER-JOIN-to-join-with-inline-json-.patch ajouté
- Fichier 0011-schemas-add-an-absent_label-property-to-Dimension-38.patch 0011-schemas-add-an-absent_label-property-to-Dimension-38.patch ajouté
- Fichier 0012-visualization-utils-remove-dead-import-38067.patch 0012-visualization-utils-remove-dead-import-38067.patch ajouté
- Fichier 0007-engine-do-not-hardcode-cube-s-json-field-name-38067.patch 0007-engine-do-not-hardcode-cube-s-json-field-name-38067.patch ajouté
- Fichier 0001-tests-load-schema2-fixture-only-one-time-38067.patch 0001-tests-load-schema2-fixture-only-one-time-38067.patch ajouté
- Fichier 0013-overhaul-of-query-to-table-transformation-38067.patch 0013-overhaul-of-query-to-table-transformation-38067.patch ajouté
- Fichier 0002-tests-use-tabulate-to-compare-tables-38067.patch 0002-tests-use-tabulate-to-compare-tables-38067.patch ajouté
Ne pas garder les dimensions NULL pour des décomptes à 0.
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
- Lié à Development #38066: utiliser uniquement LEFT OUTER JOIN pour les jointures avec les tables d'item ajouté
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
Frédéric Péters a écrit :
format_json_fixtures.py, dans le json.dump, j'ajouterais separators=(',', ': '), pour ne pas avoir d'espace en fin de ligne. (ou je forcerais à exécuter le script avec Python >= 3.4, où c'est devenu le paramétrage par défaut.
Ok.
return unicode(self.label); ce serait bien de prendre en compte Python 3 dans le nouveau code.
Je me suis dit ça mais comme il y a déjà beaucoup de unicode()
dans les mêmes fichiers j'ai préféré garder ça comme ça pour tout faire d'une seule traite après plutôt que de mélanger portage python3 et ce ticket dont je savais déjà qu'il allait devenir bordélique.
[...]
Sur une visualisation avec juste "nombre de demandes", pas de regroupements/répétitions, rien. (cell vaut [])
Pourtant là ça marche :
C'est en local ou tu l'as vu sur une instance de test ?
PS: j'ai compris, j'ai posé un patch au dessus du dernier commit qui corrige le souci je pense.
Sur le fond, je ne connais pas assez pour relire, mais ça m'irait d'avoir une petite description de ce qui a été entrepris dans ces patchs. (la description revue contient déjà un peu plus d'infos).
Je suis parti de mon constat sur #38031 : je pensais que ce que demandait Thibault était une évolution mais en fait on calcule très mal les axes des tableaux et de manière différente selon qu'on a deux axes ou un seul (avec deux axes on génère la liste des valeurs pour chaque dimension, dans le cas 1 dimension on prenait directement les valeurs retournées par la requête SQL).
Ça m'a d'abord fait ouvrir #38066, puis je me suis aperçu qu'agir sur le schéma généré par wcs-olap ne suffirait pas vu que le calcul des tables ne fonctionnait pas bien dans le cas pointé: les tables à deux dimensions ignore les NULL, les tables à 1 dimensions ignore les valeurs auxquelles ne correspondent aucun formulaire dans les deux cas c'est insuffisant; de plus le code coté vue JSON était différent augmentant les chances d'avoir des résultats différents entre bijoe et combo.
Maintentant je construis un defaultdict() "grid" dans table_2d et table_1d qui contient par défaut la mesure vide (en général 0), comme j'ai eu besoin de travailler directement sur la correspondance entre valeur de dimension et leur liste Member(id,label) il a fallu que je revienne au niveau de Engine.query() sur le fait de ne renvoyer que la valeur "value_label" pour chaque dimension, d'où l'ajout dans la génération SQL de ça :
- projections.append('%s AS %s' % (dimension.value_label or dimension.value, - dimension.name)) + projections.append('%s AS %s' % (dimension.value, dimension.name + '_value')) + if dimension.value_label: + projections.append('%s AS %s' % (dimension.value_label, dimension.name + '_label'))
aussi à plusieurs endroits (paramètre de Engine.query() ou son résultat) je passais soit le nom d'une dimension ou un extrait de ses attributs j'ai trouvé plus simple désormais de travailler directement avec les objets Measure/Dimension et de faire une différence entre les cellules pour les mesures et les dimensions.
Désormais dans cell.dimensions[*].value
on a des choses qui correspondent à member.id
renvoyé par EngineDimension.members()
plutôt que de faire correspondre sur le label (ça ne marchait plus avec la valeur NULL mappé sur "Aucun(e)" faire autrement rendait le code encore plus illisible).
- un traitement correct des dimensions dans tous les cas (on ignore plus les NULL sauf si aucun décompte ne leur est lié, on ignore plus les valeurs non-NULL mais qui ont un décompte à 0 quand il y avait un left outer join)
- on ne crée plus de colonne/ligne total si c'est inutile,
- on a accès à la définition des dimensions/mesures tout du long de la chaîne de génération des données
- les sorties en tableau, en graphique, en ODS et en JSON utilisent toute le même code
- la transformation des valeurs en chaîne est gérée au niveau engine et pas visualisation ce qui rend le code plus clair à cet endroit (plus de
stringified()
).
Les premiers commits sur les tests c'est vraiment en passant pour accélérer un peu les tests parce que j'ai du les faire tourner beaucoup pour voir ce qui changeait en bien ou pas au fur et à mesure (on gagne 2 à 3 minutes sur les 16 minutes usuelles).
Les commits autour des colonnes JSON c'est parce que pas mal de tests ont pété dans schema2 car ce sont des visualisations sur le cube "Tous les formulaires" et ça remontait des valeurs NULL en trop (ou ça les oubliait) dans les dimensions.
Je voulais modifier le moins possible mais au fur à mesure j'ai du remonter jusqu'au niveau de la génération des requêtes pour remettre les choses à plat.
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
Mis à jour par Frédéric Péters il y a plus de 4 ans
PS: j'ai compris, j'ai posé un patch au dessus du dernier commit qui corrige le souci je pense.
(oui c'était en testant la branche en local, et oui ça fonctionne désormais).
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
Le dernier patch est rebasé dans le dernier commit de la branche et la plate-forme de test est à jour sur cette même branche, si personne ne signale de régression en recette je sui d'avis de pousser; après #38066.
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
- Sujet changé de Revoir complètement la génération des tableaux et des valeurs NULL due aux OUTER JOIN à Revoir complètement la génération des tableaux et la gestion des valeurs NULL dues aux OUTER JOIN
Mis à jour par Frédéric Péters il y a plus de 4 ans
Dans #37899 j'ai tapé un commit dans combo suite à une visualisation qui était :
- regroupement vertical : formulaire
- répétition : catégorie
qui produisait une série de tableaux, genre :
Cadre de vie ------------ Démarche 1 | 12 Démarche 2 | 14 Démarche 3 | 7 Signalement ----------- Arbre | 8 Déchets | 4
Après ce ticket, le résultat est :
Cadre de vie ------------ Arbre | 0 Déchets | 0 Démarche 1 | 12 Démarche 2 | 14 Démarche 3 | 7 Signalement ----------- Arbre | 8 Déchets | 4 Démarche 1 | 0 Démarche 2 | 0 Démarche 3 | 0
Alors perso, j'étais de toute façon d'avis pour dégager "répétition", ce changement de comportement va juste mettre tout le monde d'accord là-dessus.
Mis à jour par Frédéric Péters il y a plus de 4 ans
Il manque une référence au ticket dans "apply filters to loop dimension enumeration too" et il y une partie "perf" qui s'est glissée dans "engine: propagate filters to dimension's members enumeration" et pourrait être son propre commit; à part ça je n'aurai rien à dire.
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
Frédéric Péters a écrit :
Il manque une référence au ticket dans "apply filters to loop dimension enumeration too" et il y une partie "perf" qui s'est glissée dans "engine: propagate filters to dimension's members enumeration" et pourrait être son propre commit; à part ça je n'aurai rien à dire.
Corrigé ces deux soucis, j'ai refait une passer sur les différences dans tests/fixtures/schema2/tables.json et toutes les modifications sont celles souhaitées.
Mis à jour par Frédéric Péters il y a plus de 4 ans
- Statut changé de Solution proposée à Résolu (à déployer)
Et tu as poussé dans master.
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
commit 980cb76aaea2fee09039e294e0ea1c992b8a5b6b Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Wed Dec 4 19:30:27 2019 +0100 settings: improve tests runtime by turning DEBUG off (#38067) commit 10b4405fd11aee06f8c87ae47346c235f5c54a54 Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Wed Dec 4 12:53:16 2019 +0100 engine: propagate filters to dimension's members enumeration (#38067) commit 72e8d4c83e534a5c2d2c02aa9d4b0010c95e4097 Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Sat Nov 30 03:59:00 2019 +0100 overhaul of query to table transformation (#38067) * during query obtain dimension id and label if a different projection is defined * use object to materialize query results : Cells, DimensionCell, MeasureCell * handle stringification in the *Cell classes * never ignore NULL dimension's values (it's detected in Visualization.data() and added to the list of dimension members) * sum of columns is only computed if there are more than one column * sum of rows is only computed if there are more than one row * full sum is only computed if there are more thant one column and more than one row * 1 dimension table are computed in the same maner as 2 dimensions tables, no more discrepancies * JSON web-service now use the same base methods table_2d() and table_1d() as the native rendering in bijoe * EngineDimension.members is specialized for bool dimensions (as it's always True/False) commit 6402c7ea99fea8e31634dff6f6e9f17fc393e082 Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Sat Nov 30 03:57:21 2019 +0100 visualization/utils: remove dead import (#38067) commit e8e1d573c57a90c977d1b00239040ebb13319f11 Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Thu Nov 28 20:14:16 2019 +0100 schemas: add an absent_label property to Dimension (#38067) Also force lazy strings to unicode in to_json(). commit 2422edd73341ded8809451fb35b62b5294a02f21 Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Fri Nov 29 15:31:25 2019 +0100 engine: use LEFT OUTER JOIN to join with inline json join table (#38067) commit d603f148607da2cde67ef50194be3993d9efe7fa Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Fri Nov 29 15:31:08 2019 +0100 engine: hide NULL values of json dimensions in inline join table (#38067) commit 47015684372edfe0a4e53bbe7d717e4bd47fe306 Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Fri Nov 29 15:29:48 2019 +0100 engine: cache proxy descriptor result on engine's cubes (#38067) commit 3f7bedf0d317324eed7af0e32194455172448a64 Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Fri Nov 29 15:29:04 2019 +0100 engine: do not hardcode cube's json field name (#38067) commit a868f11d42b31e38536671ef4b4dd9afd7928abd Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Fri Nov 29 15:27:22 2019 +0100 engine: cache EngineDimension.members (#38067) commit 33a8eabdba2afba00cdd44146e63889eacf3f704 Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Fri Nov 29 15:24:35 2019 +0100 utils: cache get_warehouses() result based on schema files paths (#38067) commit 9cc619ee0d0b3257e77ed3beb6bb371bbf6a9f36 Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Sat Nov 30 03:53:09 2019 +0100 tests: add option to update tables during tests (#38067) If table contents change, we can measure the change and eventually accept by copying new_table.json to tables.json. To activate it, use --bijoe-store-table . commit 65a78b93dcfdd0bbb305d4674bda163ee8d2b6da Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Fri Nov 29 01:35:15 2019 +0100 tests: reformat json files for easier update (#38067) Add script format_json_fixtures.py to canonicalize every JSON files in fixtures to simplify management of changes in visualization results. commit 0a612f24aca40c403fc3a7964e44134cc156270c Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Sat Nov 30 03:52:14 2019 +0100 tests: use tabulate to compare tables (#38067) Comparing strings with assert gives better diffs thant comparing list of lists when using the pytest's `-vv` option. commit 7f4f373e89e01acca12362aebca42ec8715736cc Author: Benjamin Dauvergne <bdauvergne@entrouvert.com> Date: Fri Nov 29 15:26:08 2019 +0100 tests: load schema2 fixture only one time (#38067) Use special db fixture (using pytest-django primitives and transaction.atomic()) to initialize bijoe visualization only one time for all tests on the schema2 fixture. It improves tests run time.
Mis à jour par Frédéric Péters il y a plus de 4 ans
- Statut changé de Résolu (à déployer) à Solution déployée
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
- Dupliqué par Bug #38052: KeyError at /visualization/45/json/ (u'None', u"Grand Chamb\xe9ry Cours d'eaux") ajouté
tests: load schema2 fixture only one time (#38067)
Use special db fixture (using pytest-django primitives and
transaction.atomic()) to initialize bijoe visualization only one time
for all tests on the schema2 fixture. It improves tests run time.