explorable-viz / fluid

Data-linked visualisations
http://f.luid.org
MIT License
36 stars 2 forks source link

Merge records with dictionaries #1076

Closed rolyp closed 1 week ago

rolyp commented 2 months ago

As long as Fluid is untyped, the question arises of whether the language really need both constructs. Although it’s true that field names are not first-class and dictionary keys are, it’s unclear why we can’t freely treat strings as field names. In JavaScript for example one can use the notation e.x when x is an identifier and the notation e[e'] when e' is an expression of type string, with a similar convention for dictionary literals. We could adopt a similar approach and do away with the distinction between records and dictionaries.

Tentative refactoring plan:

  1. Generalise field access notation e.x to work with dictionaries in the e position; migrate dict_get examples to use the new notation whenever the key happens to be a string
  2. Generalise field access notation to e.e' where e' is arbitrary string-valued expression; migrate remaining dict_get examples and delete dict_get
  3. Generalise record pattern-matching to work with dictionaries; dictionaries have selection information on keys as well as the values, so take that into account in a way that preserves the current record-matching behaviour
  4. Change parser to map record notation to dictionaries; use to validate pattern-matching; remove record expressions from the abstract syntax
  5. Generalise record notation to support arbitrary expressions in “field” position, as per dictionary literals; maybe borrow [ e ] notation from JavaScript
  6. Migrate examples involving dictionary notation to record notation; remove support for dictionary literals from the parser
  7. Delete dict_fromRecord

We’d have to refine this plan a bit to take into account desugaring. We may want dictionary update at some point too.

rolyp commented 2 months ago

@JosephBond I’m thinking that this should probably be one of the near-term implementation tasks, as it will significantly improve our ability to write “generic” code for operating on records, whilst retaining the nice readable record syntax, which will be crucial for convincing examples. I think this can be done as a (roughly) 5-step refactoring that exploits the asymmetry between introduction and elimination forms: (1) generalise record elimination forms to support dictionaries; (2) modify record introduction form to produce dictionaries; (3) generalise record introduction form to support arbitrary expressions in place of static keys; (4) migrate examples that use dictionary notation to the new generalised records; (5) remove support for records. I’ve sketched out a bit of plan in the body of the issue.

JosephBond commented 2 months ago
rolyp commented 1 month ago

Hi @JosephBond. Slight tweak to yesterday’s plan: changing the record parser to build dictionaries directly will probably break the round-tripping, so I think it makes more sense to implement this change in the desugaring instead. Then the original syntax can be recovered during desugarBwd and pretty-printed. That will leave us with two “notations” for dictionaries:

with the former (essentially) desugaring into the internal representation of the latter.

We would really like to lose the = in the final syntax. This is present in the dictionary notation because e : e’ would parse as cons. However this should not be a problem if the “expression key” form is wrapped in square brackets. So the next step would I think be to verify that we can change the dictionary grammar to lose the =:

Then we can think about the next step, which will either be extending record notation to support [ e ]: or dictionary notation to support x: (not sure yet which direction makes the most sense).

rolyp commented 2 weeks ago

@JosephBond Actually the problem with the GitHub Actions failing was actually because of an incorrect URL for one of the Puppeteer tests, so I don’t think we have a performance problem that’s impacting tests after all. (Performance is somewhat degraded with first-class keys everywhere, but not in a way that’s disrupting testing.)

There’s a flag called puppeteerLogging which revealed this, but I’ve changed things so that the top-level test URL is always logged (regardless of the value of the flag), since currently this is specified manually and if wrong, will cause the tests to silently fail.