-
Notifications
You must be signed in to change notification settings - Fork 40
Description
In #664 (specifically packages/catlog/src/stdlib/analyses/ecld/atomisations.rs), we've come across a case where the choice between ustr and uuid is slightly annoying: part of this analysis (degree_atomisation()) creates new objects, and those new objects need new identifiers. An easy option makes sense to simply use Uuid::now_v7(), but this presents two problems:
-
We can't simply use our test model that we define in
catlog/src/stdlib/modelsbecause this is defined withustr, and even I figured out how to do some fancy type parametrisation to let thedegree_atomisationfunction take this as an input, there is (as I understand) no analogue ofuuid::now_v7()forustr, so we'd have to have some model with mixed identifier types. Not only this, the test wouldn't be so simple to implement: if we also predefine what the outcome of this "add some new variables and arrows" should be then we can't test for equality with the output of thedegree_atomisationsince the uuids will be different! In other words, for testing purposes, ustr really does make sense. -
It feels like we're throwing away some useful information, because the objects that we're creating here are not just arbitrary objects. In fact, all of them "should" be identified in a way that makes it clear how they were created: all of them are "derivatives" of a user-defined object, i.e. for each variable
$x$ we create objects$x^{(1)},x^{(2)},\ldots,x^{(n)}$ (for some$n$ determined by the incoming and outgoing arrows of$x$ ).
I know that @epatters and @olynch have been thinking about this, so I'm hoping that you can just tell me what the good thing to do is here (in terms of what identifiers should look like), but here's my naive suggestion: we take the product type, so that an identifier is always a uuid and a ustr; the analogue of uuid::now_v7() is simply (uuid::now_v7(), ""); we always test for equality on the uuid component unless we tick a magic box somewhere that says to ignore that and test on the ustr component instead.
This suggestion feels particularly weird because I'm pretty sure that @epatters mentioned in passing that we should build some enum (coproduct), not a product.
(Maybe relevant to #58)