digitaltwinconsortium / DTDLParser

Digital Twin Definition Language parser for .NET
MIT License
14 stars 8 forks source link

Feature proposal: GetImplicitElements method #93

Closed jrdouceur closed 1 year ago

jrdouceur commented 1 year ago

Proposal to add a new method to ModelParser:

public IReadOnlyDictionary<Dtmi, DTEntityInfo> GetImplicitElements()

This would be analogous to the extant method:

public IReadOnlyDictionary<Dtmi, DTSupplementalTypeInfo> GetSupplementalTypes()

The new method would provide an object model that includes all elements implicitly defined by DTDL, by the standard extensions, and by any partner or feature extension that is directly supported by the parser. Eventually, whenever dynamic extensions are supported, the returned object model will also include elements defined by any extension that has been loaded into the ModelParser instance.

rido-min commented 1 year ago

I have some observations:

  1. I see the same results when using MaxDtdlVersion = 2 . I expected elements matching what is available in each version.
  2. Instead of returning all implicit elements, we could split this in different methods, eg: by standard, instance and by extension.
  3. In the same way we added break out properties, will be good to provide a better object model to navigate these results, without forcing the "query+casting" pattern.

Could not find an easy way to answer original question: fetch all the units for Acceleration in #92

jrdouceur commented 1 year ago
  1. I see the same results when using MaxDtdlVersion = 2 . I expected elements matching what is available in each version.

This seems to be an observation regarding the expectations of MaxDtdlVersion. At present, the only effect of this option is to reject models that specify a DTDL context version exceeding this value. In other words, this comment does not relate only to the proposed GetImplicitElements() method but rather to the parser's full behavior in regard to the MaxDtdlVersion option. If you believe the parser should provide stronger semantics for this option, please open an issue to this effect. This is a highly non-trivial change.

  1. Instead of returning all implicit elements, we could split this in different methods, eg: by standard, instance and by extension.

Yes, this is possible. It would be similarly possible to split the extant GetSupplementalTypes() method into multiple methods that partition the types by extension. In the interest of parallelism between the types and the elements, it seems to me that either both methods should be split in this manner or neither should be. Implementing the split is significant additional work because the internal data structures do not at present partition the type and element information in this way.

  1. In the same way we added break out properties, will be good to provide a better object model to navigate these results, without forcing the "query+casting" pattern.

Are you referring to the casting to DTEnumValue in Tutorial13?

Could not find an easy way to answer original question: fetch all the units for Acceleration in #92

The additions to tutorials 12 and 13 illustrate how fetch all units for Distance (because this type was already used in the tutorial). The answer for Acceleration is a straightforward adaptation of the tutorial code.

rido-min commented 1 year ago
  1. I'll add a new issue wrt MaxDtdlVersion behavior
  2. Agreed, should be good to have the parallelism between types and elements.
  3. Tutorial 13 seems complicated to me, as it requires in-depth knowledge of the underlying DTDL concepts. I was thinking of a higher API that encapsulates how to access supplemental types and units. eg, to get all units for semantic types, I have to filter by EntityKind =Enum and then cast to DTEnumInfo
var implicitElements = new ModelParser().GetImplicitElements();
foreach (var implicitElement in implicitElements
    .Where(i => i.Value.EntityKind == DTEntityKind.Enum && i.Value.Id.OriginalString.StartsWith("dtmi:dtdl:extension:quantitativeTypes"))
    .Select(t => (DTEnumInfo)t.Value))
{
    Console.WriteLine($"{ModelParser.GetTermOrUri(implicitElement.Id)} {implicitElement.Id}");
    foreach (var item in implicitElement.EnumValues)
    {
        Console.WriteLine($"  {item.Name}");
    }
}  

I would rather prefer something like:

var semanticTypes= new ModelParser().GetSemanticTypes();
foreach (var st in semanticTypes)
{
  Console.WriteLine(st.Name);
  foreach (var unit in st.Units)
  {
    Console.WriteLine($"  {unit.Name}");
  }
}

2 and 3 are very related and might lead to the same solution.

I would not use the implementation effort to drive the best API design.

jrdouceur commented 1 year ago

I did not mean to imply that I think we should avoid a good design because of the work it involves. I agree on the value of a easy-to-use solution.

  1. At present, there is no way whatsoever to access the unit information or any other non-type-level language or extension information via the parser API.
  2. At present, we have a method to access supplemental type information (which could be improved at some point to make it more usable), but we have no corresponding method for accessing model-level information.

We can queue up work to both (1) add a highly usable mechanism for accessing units and (2) improve the usability of the type-accessing mechanism, but I don't know either (a) how highly we should prioritize these relative to other work or (b) how quickly we could get these done once the work is started, since this are non-trivial efforts.

In view of the above, my inclination is to provide a mechanism now that (1) provides a perhaps non-ideal mechanism for accessing all model-level information from the metamodels via the parser, to enable use cases that are currently not enabled at all, and that (2) is parallel with the currently offered mechanism for accessing supplemental type information. If we do not do this, I expect it will be quite a while before these use cases are achievable.

rido-min commented 1 year ago

Ok, let's publish GetImplicitElements

Can we add a "manual wrapper" to implement GetSemanticTypes and Units?

jrdouceur commented 1 year ago

You mean like in a sample? The way we did with ParseToJson? This seems probably doable. I'll think on it a bit.