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

Feature proposal: GetImplicitElements method #93

Closed
jrdouceur opened this issue Mar 10, 2023 · 6 comments · Fixed by #94
Closed

Feature proposal: GetImplicitElements method #93

jrdouceur opened this issue Mar 10, 2023 · 6 comments · Fixed by #94
Assignees
Labels
enhancement New feature or request

Comments

@jrdouceur
Copy link
Collaborator

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.

@jrdouceur jrdouceur added the enhancement New feature or request label Mar 10, 2023
@jrdouceur jrdouceur self-assigned this Mar 10, 2023
@jrdouceur jrdouceur linked a pull request Mar 10, 2023 that will close this issue
@rido-min
Copy link
Collaborator

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
Copy link
Collaborator Author

  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
Copy link
Collaborator

  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
Copy link
Collaborator Author

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
Copy link
Collaborator

Ok, let's publish GetImplicitElements

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

@jrdouceur
Copy link
Collaborator Author

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

@rido-min rido-min added this to the First GA. Version 1.0 milestone Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants