Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom primitive types / built-in types #156

Closed
alessiostalla opened this issue May 15, 2024 · 14 comments · Fixed by #158
Closed

Custom primitive types / built-in types #156

alessiostalla opened this issue May 15, 2024 · 14 comments · Fixed by #158

Comments

@alessiostalla
Copy link
Contributor

I see in deserializeBuiltin there is no way to deserialize a custom type while Lionweb/Java supports this (https://github.com/LionWeb-io/lionweb-java/blob/8b7bb5d9dee3f9687ea6847c72afb1aea85847b1/core/src/main/java/io/lionweb/lioncore/java/serialization/PrimitiveValuesSerialization.java#L56-L60).
We now have encountered models serialized with that feature from Java that we cannot deserialize in JS.
Is this planned? Is there a workaround?

@dslmeinte
Copy link
Contributor

I think the ExtractionFacade would be a good place to put that kind of configurability. (In the future, that interface will probably be renamed to ReflectionFacade or something similar.) No concrete plans, at the moment. I think I'd first want to do the re-architecting before adding this kind of functionality in.

@alessiostalla
Copy link
Contributor Author

Could we help in some way to get this feature in quicker?

@dslmeinte
Copy link
Contributor

In many unrealistic and impractical ways...

Are you working on a concrete use case implementation that needs this?

@alessiostalla
Copy link
Contributor Author

Yes, our ASTs have source position information objects that used to be represented as nodes in Lionweb but that would be too much for the model repository to handle for some larger ASTs. So we switched to representing them as attributes. However, now we cannot deserialize those models in our web application anymore, and as far as I know we cannot work around it (except by reimplementing the import from JSON).

@dslmeinte
Copy link
Contributor

(btw: I think here you should be using the key instead of the ID.)

@dslmeinte
Copy link
Contributor

I think the mechanism of registering serializers (and possibly deserializers) for specific primitive types (identified by meta-pointer should not be very difficult to add to the TS implementation as well, and its implementation would have much interference from the re-architecting, so I might try and make some time to have a stab at it this week(-ish).

Keep bugging me to ensure ;)

@ftomassetti
Copy link
Contributor

(btw: I think here you should be using the key instead of the ID.)

I have added an issue to track that: LionWeb-io/lionweb-java#141

@enikao
Copy link
Contributor

enikao commented May 16, 2024

According to LionWeb-io/specification#10, we don't support custom primitive types yet.
In theory, you could use JSON as primitive datatype -- that should work, but I think is not implemented on all platforms.

@dslmeinte
Copy link
Contributor

Good point! Having support in the main two implementations might give the wrong idea, even.

On the other hand: people can define their own primitive types in their languages (and we didn't curb that capability as per LionWeb-io/specification#10).

I wouldn't mind at least improving the (de-)serialization of values of properties having a primitive type a bit (because it's a bit...happenstance, right now), and then some kind of parametrization is also easier.

@ftomassetti
Copy link
Contributor

In theory, you could use JSON as primitive datatype

My understanding is that ultimately every primitive value ends up being serialized as a string into a "LW JSON file". The indication of the primitive type just tells us how to interpret that string. For example, "10" may be become 10, if the primitive type is int, while it could stay "10", if the type is a string.

Suppose someone wants to use LocalDates. If they use JSON to represent that, it means that values will be unserialized as JSON (for example, instances of JSONElement in Java). That would be way less convenient that having instances of LocalDates, for example. Basically all operations will require to convert back and forth between LocalDate and JSONElement, to ultimately store JSONElements values in the node.

According to LionWeb-io/specification#10, we don't support custom primitive types yet.

On the other hand: people can define their own primitive types in their languages (and we didn't curb that capability as per LionWeb-io/specification#10).

This means that we can define primitive types, but if we use them, serialization/deserialization should fail.

@enikao
Copy link
Contributor

enikao commented May 20, 2024

To me, there's one main advantage of our JSON primitive type: As a user (i.e. developer using e.g. Java), I'd get it as JSONElement -- LW infrastructure would take care of proper unescaping, parsing, error handling, etc. LW does not give that data more semantic meaning.
Assume we parse files to get our AST. We wanted to store the source file, line, column, and total index as a tuple for each node.
Then storing it as JSON relieves us from coming up with a "serialization format" for these 4 data points, and converting line/column/total index from string to int.
With my recent value types proposal (LionWeb-io/specification#265) we'd even get a semantic representation of that "Coordinate".

Looking at LocalDate, the story is different. Most programming languages already have some way of representing it. If we wanted interoperability, we had to come up with a language definition to represent the underlying data, and a "gentlemen's agreement" that every host language would have special handling of that language to represent it in the proper way for the host language.
Whether we serialized LocalDate as 2024-05-25 or { \"year\": 2024, \"month\": 5, \"dayOfMonth\": 25 } does not really matter.

I read LionWeb-io/specification#10 as postponed, i.e. not decided yet (yes, the text is a bit ambiguous). I'm not sure to what extent custom datatypes are supported in all implementations (C# doesn't support it atm).

@ftomassetti
Copy link
Contributor

Assume we parse files to get our AST. We wanted to store the source file, line, column, and total index as a tuple for each node.

Indeed we are using a custom primitive type to represent the Position of an AST node in a file (as a pair of Line and Column). We want to do that because the class Position has many convenient methods.

Let's say we have nodes with positions expressed through a specific class:

class MyNode { 
  position: Position;
}

If I want to check if one node comes before another one, I can do that easily:

if (nodeA.position.isBefore(nodeB.position)) { ... }

If I represent it as JSONElement:

class MyNode { 
  position: JSONElement;
}
// extra helper method I now need
Position toPosition(jsonElement: JSONElement) { ... }

if (toPosition(nodeA.position).isBefore(toPosition(nodeB.position))) { ... }

I need to do the conversion from JSONElement to position before invoking the operation. Also, I have code that is more error prone as certain JSONElements could be converted to positions, while others would be representing other things.

Without custom primitive types, it seems to be that:

  1. We lose information (we conflate many different things into JSONElements)
  2. We end up with code that is more error-prone
  3. We end up with code that is less convenient to write

Also, this is asymmetric to what we do for node classes: for node classes, we can specify specific classes to be instantiated when unserializing nodes. It would be useful to have the same approach for primitive types.

Then storing it as JSON relieves us from coming up with a "serialization format" for these 4 data points, and converting line/column/total index from string to int.

Yes, but it sort of "partially unserialize" because it goes from String to JSONElement, but not to the final data types that would make the processing convenient (e.g., instances of the class Position).

@enikao
Copy link
Contributor

enikao commented May 20, 2024

It would be useful to have the same approach for primitive types.

I agree completely --> LionWeb-io/specification#265

@dslmeinte
Copy link
Contributor

I think some wiggle room exists between fully supporting custom primitive types (e.g., guaranteed and specified plugin points in the [de-]serializer in our own reference implementations) and fully forbidding custom primitive types (in which case the only instances of PrimitiveType would be in LionCore). We only specify the serialization format, not the (de-)serializer, so everyone's free to convert a string (or a JSON element) to instances of appropriate classes during deserialization and vice versa. It doesn't even make those chunks “less exchangable” since either you'd be interested in that information and would rather make the effort, or you're just going to ignore it.

Currently, I feel that we should align our stance on custom primitive types, and maybe clarify that (FAQ-style?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants