Produits Entr'ouvert » Combo

Ça casse le |add pour les usages non-arithmétiques, qu'on a par exemple dans des intégrations graphiques,

url({% asset_url "wcs:category:logo-active:"|add:cell.category_reference %});

• pour éviter que le add de django accepte 0.3 et le convertisse en 0
• pour convertir 'not a number' en 0 si l'autre terme est convertible en décimal

mais je réalise que l'on pourrait aussi vouloir concaténer un nombre à une chaîne, ce qui ne fonctionnera plus (ex: 'version' + '0.3').

Peut-être donner un nouveau nom comme "addition" pour l'addition arithmétiques ?

J'ai l'impression que de renommer "add" en "concat" dans les intégrations graphiques, ne suffirait pas puisque d'autres occurrences peuvent exister en base : cellules prototype json...

(non ne mettons pas un templatetag différent)

• les cellules prototype JSON
• les jsondatastores
• les requêtes csv et depuis peu opengis, arcgis

Est-ce que j'ai fait le tour ?
Auquel cas, je pourrais les adapter, ainsi que les intégrations graphiques afin d'intégrer le même opérateur que dans wc.

Ce n'est pas réaliste du tout d'imaginer pouvoir tout modifier.

Nicolas Roche a écrit :

Pour revenir sur https://dev.entrouvert.org/issues/41868#note-4,
je me demande où je devrais chercher pour voir où d'autre le filtre add serait utilisé par les utilisateurs,
et en fin de compte j'ai l'impression que les gabarits ne sont pas souvent utilisés :

• les cellules prototype JSON
• les jsondatastores
• les requêtes csv et depuis peu opengis, arcgis
  Est-ce que j'ai fait le tour ?

Absolument à tout bout de champ dans wcs :)

La règle est assez simple : il ne faut pas toucher aux comportements existants.

Notre erreur dans wcs est d'avoir créé un "add" qui écrase celui qui existe dans Django. On n'y touchera pas, c'est pas très grave. Mais le add de Django est déjà utilisé par des usages de Combo, et donc on ne peut plus le remplacer.

Je propose, pour s'autoriser un "add" dans Combo : ne faire l'addition que si le membre de gauche est déjà un Decimal ; sans tenter de conversion. Et sinon, fallback vers le "add" natif de Django.

Cela permettra de faire « foo|decimal|add:bar », ça reste compréhensible.

Thomas Noël a écrit :

Je propose, pour s'autoriser un "add" dans Combo : ne faire l'addition que si le membre de gauche est déjà un Decimal ; sans tenter de conversion. Et sinon, fallback vers le "add" natif de Django.

Et je relis la doc de Django et c'est déjà ce que ça fait, en fait... il ne faut rien faire pour add :)

(le truc qui pousse Nicolas ici c'est qu'on a une conversion de la virgule comme séparateur décimal)

Frédéric Péters a écrit :

(le truc qui pousse Nicolas ici c'est qu'on a une conversion de la virgule comme séparateur décimal)

Ok alors je n'ai pas trop d'avis s'il faut ou non conserver la magie de conversion. Si on veut conserver la magie, je serais pour ne faire le do_raise que sur le membre de gauche.

Aussi, le « from django.template.defaultfilters import add as native_add » est inutile, utiliser directement defaultfilters.add déjà disponible.

Règles de #42062 à appliquer ici. (si ce n'est déjà le cas)

Il n'y a pas de notion de "unlazy" dans Django, c'est un truc lié à w.c.s., tu peux les retirer ici.

Le parse_decimal pourrait retourner None en cas d'impossibilité de conversion. Alors dans decimal, tu ferais « value = parse_decimal(value) or Decimal(0) ». Et tu éviterais tout un tas de try/except dans le add:

...
term1_decimal = parse_decimal(term1)
term2_decimal = parse_decimal(term2)

if term1_decimal is not None and term2_decimal is not None:
    return term1_decimal + term2_decimal
if term1 == '' and term2_decimal is not None:
    return term2_decimal
if term2 == '' and term1_decimal is not None:
    return term1_decimal
return defaultfilters.add(term1, term2)

Il n'y a pas de notion de "unlazy" dans Django

aïe, j'ai honte.

Le parse_decimal pourrait retourner None en cas d'impossibilité de conversion.

bof, ce même code est déjà poussé sur wcs et ça casse pas mal l'existant, ne serait-ce que parce que les autres opérateurs utilisent aussi parse_decimal().

    def test_decimal_templatetag():
        tmpl = Template('{{ plop|decimal }}')
>       assert tmpl.render(Context({'plop': 'toto'})) == '0'
E       AssertionError: assert 'None' == '0'
E         - 0
E         + None

Le parse_decimal pourrait retourner None en cas d'impossibilité de conversion.

bof, ce même code est déjà poussé sur wcs et ça casse pas mal l'existant

Très compliqué pour moi de comprendre si "ça casse" parle du code qui a été poussé dans w.c.s., et si "l'existant" parle de code/conditions/gabarits qui y existent; ou si "ça casse" est plutôt une réponse "non" à Thomas, que sa proposition "casserait" "un existant" (même inconnue ici).

Le code dans wcs a sa vie et son existant, qui nous a obligé à jouer les affaires autrement, laissons-le tranquille, n'y touchons plus, il roule. Mais ici parse_decimal est tout neuf donc y'a aucun soucis, et je trouve que le code de "add" avec juste des "if" serait bien plus lisible.

J'étais parti du principe que l'on faisait juste du copier/coller des filtres entre combo et wcs, pour faciliter leur maintenance (c'est du moins le discours que j'ai entendu à plusieurs reprises).
Partant de là, comme le code de parse_decimal est utilisé sur wcs par les opérateurs arithmétiques d'une part, mais aussi dans les gabarits django, ma réponse était "plutôt non", si l'on veut conserver la gestion transversale (ou bête) du code des filtres entre les deux briques.

• decimal()
• substract()
• multiply()
• divide()
• ceil()
• floor()
• abs()

• add_days()
• add_hours()

J’avertis que l'on a (selon moi) changé de position et demande un avis tranché de votre part.

J'étais parti du principe que l'on faisait juste du copier/coller des filtres entre combo et wcs, pour faciliter leur maintenance (c'est du moins le discours que j'ai entendu à plusieurs reprises).

On ne peut pas faire de copié/collé il y a tout le concept "lazy" qui existe dans wcs et pas dans combo. Si ça a été dit, faut l'oublier, ça n'est pas possible.

Nicolas Roche a écrit :

J'étais parti du principe que l'on faisait juste du copier/coller des filtres entre combo et wcs, pour faciliter leur maintenance (c'est du moins le discours que j'ai entendu à plusieurs reprises).

Oui c'est vrai, on dit ça... mais la réalité nous rattrape ici. Dans w.c.s. on avait un existant sur parse_decimal qu'on a évité de bouleverser. Sur Combo on les coudées franches, et je préférerais alors un add plus "joli".

Mais déjà, notons qu'un truc chouette ça va être de copier/coller les tests. Vu qu'ils couvrent tout ce qui nous intéresse, c'est déjà super.

Nicolas Roche a écrit :

Oui, les tests sont identiques des deux côtés.

Cool.

Bon, je vais t'entendre soupirer "rahhh ces relecteurs...", mais si on fait :

   def parse_decimal(value, default=Decimal(0)):
       ...
       except (ArithmeticError, TypeError):
           return default

alors dans "add" on utilisera des « parse_decimal(..., default=None) », et pour les autres opérateurs le code sera, ô victoire, celui de w.c.s :)

(et ensuite vraiment je pense que ça va être un ack, oui oui)