Specification Tagging #3113
Specification Tagging #3113
Conversation
🦋 Changeset detectedLatest commit: 74c4162 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
| spec = Specification.make( | ||
| { | ||
| title: "Stock market over time", | ||
| description: "The S&P500 stock market price, over time.", |
There was a problem hiding this comment.
Can we move title and description to tags? Same as with Plot.*, which had title: initially and now they don't.
There was a problem hiding this comment.
I'm fine moving description to tags. I'm nervous about title, because that should probably be mandatory. Seems weird to have a mandatory tag.
| ], | ||
| frSpecification, | ||
| ([{ title, description, validate, showAs }]) => | ||
| vSpecification({ |
There was a problem hiding this comment.
(minor)
frSpecification defines pack, so could this be a plain {...} object instead of vSpecification({...}.value?
| }), | ||
| }; | ||
| }, | ||
| { isDecorator: true } |
There was a problem hiding this comment.
Do you know why this doesn't work as a decorator?
I couldn't figure it out yet but it fails frSpecification check: playground
There was a problem hiding this comment.
Not yet. It seemed to be something internal, maybe because it's a function or something.
| return { | ||
| value, | ||
| tags: tags.merge({ | ||
| specification: vSpecification(spec), |
There was a problem hiding this comment.
Maybe hard-fail if the value already has a spec, since we don't support multiple specs per value yet?
More explicit, and it's always possible to erase the previous spec with Tag.omit(...).
There was a problem hiding this comment.
Hmm, I've just realized that tags semantics might make some things awkward.
Whenever you modify a value, you lose all tags on it. So if you want to tune someone's forecast, you'll need to reattach some relevant tags, but not all of them:
@specification(originalForecast -> Tag.getSpecification)
... // any other relevant tags that you need, but usually not `@name` or `@doc`
// maybe `@format` and `@showAs` and `@startOpen`
myForecast = originalForecast -> truncateLeft(2025)
It's almost like tags should have meta-level flags that would guide whether the tag should usually be copied or erased...
There was a problem hiding this comment.
I guess we'll solve this with meta-level tag-copying functions when some common patterns emerge:
originalForecast -> truncateLeft(2025) -> Tag.copyCommonTagsFrom(originalForecast).
packages/squiggle-lang/src/fr/tag.ts
Outdated
| value, | ||
| tags: tags.merge({ | ||
| specification: vSpecification(spec), | ||
| name: vString(spec.title), |
There was a problem hiding this comment.
I'm not sure about this. Sometimes you might want to export multiple forecasts on the same spec, or import multiple forecasts from different people.
I know you can override it with @name, but you'd want @name("my forecast " + Tag.getName(spec)), which is repetitive.
Maybe we could keep the var name and show spec name next to spec widget? So instead of this:
It'd look like this:
There was a problem hiding this comment.
We often don't have much horizontal space - the value can get quite small, especially when nested.
It seems like this is an issue when:
- The user doesn't set a different name, with a tag.
- The user would prefer to see the variable name, instead of the spec name.
This seems less likely to me than them preferring to see the spec name.
| title: "Stock market over time", | ||
| description: "The S&P500 stock market price, over time.", | ||
| validate: validate, | ||
| showAs: {|e| e(Date(2024))}, |
There was a problem hiding this comment.
This example (storybook link) makes me think again about how showAs is too powerful/magical/non-transparent. And, more generally, about the tradeoffs between "output looks nice and clean" and "it's easy to figure out what's going on".
When I'm looking at the viewer, it's not clear what's going on; 2024 is hidden in the source code; tagged fn is called "Stock market over time" but the chart itself is not over time but a slice for 2024; x axis not not labeled so it's not even clear that 0-10-20 labels are not time.
I know most of this could be fixed with more careful @doc and Plot calls, but I'm not complaining about this particular example, I'm more concerned about things being interpretable by someone who hasn't read through the model carefully, and who might not even have deep enough understanding of Squiggle to read through the model, or maybe doesn't have time or attention to allocate; most models in the wild won't be carefully coded, and I want Squiggle Viewer to do the right (most clear, most interpretable) thing by default.
Specific idea (out of scope for this PR): when the most powerful version of @showAs is applied, which doesn't just customize axes but applies an arbitrary transformation of the value, indicate that explicitly in the viewer with an icon, and allow users to fall back to the original non-transformed value by clicking that icon.
There was a problem hiding this comment.
How about we just remove showAs for now? It doesn't add too much value ( could always have it be imported manually), and is confusing, as you point out.
| isEqual(other: VSpecification) { | ||
| return ( | ||
| this.value.title === other.value.title && | ||
| this.value.description === other.value.description |
There was a problem hiding this comment.
I think it makes sense to treat specs similar to JS Symbols, and Spec.make() == Spec.make() should be false.
So I'd suggest a plain identity comparison here, this.value === other.
packages/squiggle-lang/src/fr/tag.ts
Outdated
| [frWithTags(frAny({ genericName: "A" })), frSpecification], | ||
| frWithTags(frAny({ genericName: "A" })), | ||
| ([{ value, tags }, spec], reducer) => { | ||
| const showAs = spec.showAs && reducer.call(spec.showAs, [value]); |
There was a problem hiding this comment.
Is this necessary?
Possibly I'm missing something, but couldn't you copy spec.showAs to the value tag, instead of calling it here?
There was a problem hiding this comment.
I tried this and it didn't work in your example, because the original @showAs is more strict in its current signature than the version in your example.
But then it seems surprising/inconsistent to relax that limitation in this specific situtation, because the limitation was intentional? If we want to allow a generic @showAs(any) => any then we should allow it everywhere.
(see also: my another comment on how @showAs is too powerful)
Simple functionality to allow for
Specifications, with a newspecificationtag.This is very experimental for now - we won't document for public users yet, but this seems good for us to start using.
I didn't add tests in squiggle-language because there's not much real functionality there. (Specification objects hold a bit of information, but don't really do hing novel)
I'd note that we might not want "title" and "description" to be part of this object - but instead, to be added by Tags.