Page MenuHomePhabricator

ADR for syntax and placement of transform specs in Charts .chart JSON and/or invocation
Closed, ResolvedPublic

Description

Breaking out from T385540: we need to decide whether to allow transforms to be specified on the {{#chart:}} invocation directly, or only via the Data:.chart JSON itself. Secondarily we need to decide on the syntax to use in each place.

(earlier versions of this task referred to "filters", we have gone with "transforms" as it's more general and lets us reserve the term 'filters' for possible formatting extension use later.)

Decision point 1:

  • Is there only a single transform called, or do we allow a chain of multiple transforms?
  • _Outcome_: provisionally going for a single transform, which can load and call other functions if it wishes

Decision point 2:

  • We _will_ definitely have a place to invoke a transform in the Data:.chart JSON.
  • Should we also allow specifying a transform on the parser function invocation?
  • If we have both places but a single transform, is that an override?
  • _Outcome:_ allow arguments to be overridden by the invocation, but not the transform itself. That lives as part of the format definition.

Decision point 3:

  • What syntax should we use in the JSON?
  • _Outcome_: provisionally, "transform": { "module": "Module_name", "function": "function_name", "args": { "arg1": "val1", "arg2": "val2" } }
  • What syntax should we use for args in the parser function, if allowed?
  • _Outcome_: provisionally, {{#chart:Foo.chart|arg1=val1|arg2=val}}
    • Should we exclude data, width, and height used by {{#chart:}} itself, or pass them through?

Decision point 4:

  • Do we need to support null values in tables? This is awkward with the PHP-JSON<->Lua bridging and also needs specific support in the Charts renderer anyway (cf T384341)
  • _Outcome_: UNDECIDED. need to check on this still!
    • Recommend doing a spike test with a sigil replacement to make sure it works cleanly.

Decision point 5:

  • How should transforms be exported via the action API? Since this will be used to do the fetching it must be able to provide timestamps and dependencies for cache invalidation.
  • _Outcome_: Provisional example in the ADR in progress on the patchset.
    • extend action=jsondata with transform parameter, and add metadata to that data set

As this gets tested out on T388434, dropping the ADR into Chart:
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Chart/+/1130651

Event Timeline

CCiufo-WMF moved this task from Backlog to Up Next on the Charts board.
CCiufo-WMF moved this task from Up Next to Sprint 18 on the Charts board.
CCiufo-WMF edited projects, added Charts (Sprint 18); removed Charts.

@bvibber to write a patch to add this to the Charts extension in the docs/adr folder. We are hoping to get this one done today.

Change #1130651 had a related patch set uploaded (by Bvibber; author: Bvibber):

[mediawiki/extensions/Chart@master] Provisional ADR for Scribunto Lua filtering on Charts+JsonConfig

https://gerrit.wikimedia.org/r/1130651

CCiufo-WMF lowered the priority of this task from High to Medium.Mar 24 2025, 6:33 PM
CCiufo-WMF moved this task from Sprint 18 to Current Sprint on the Charts board.
CCiufo-WMF edited projects, added Charts (Current Sprint); removed Charts (Sprint 18).
CCiufo-WMF moved this task from Incoming to Code Review on the Charts (Current Sprint) board.
bvibber renamed this task from ADR for syntax and placement of filter specs in Charts .chart JSON and/or invocation to ADR for syntax and placement of transform specs in Charts .chart JSON and/or invocation.Mar 31 2025, 8:01 PM
bvibber updated the task description. (Show Details)

Updated the provisional ADR on https://gerrit.wikimedia.org/r/1130651 -- additional review on this welcome! I hope to land the ADR soon and get back to the implementation at the end of this month.

Change #1130651 merged by jenkins-bot:

[mediawiki/extensions/Chart@master] ADR for Scribunto Lua transforms on Charts+JsonConfig

https://gerrit.wikimedia.org/r/1130651

Jdlrobson-WMF moved this task from Up Next to Current Sprint on the Charts board.
Jdlrobson-WMF edited projects, added Charts (Current Sprint); removed Charts.