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

An LDD developer desires to inherit a class/attribute defined using DD_Associate_External_Class #324

Closed
jshughes opened this issue Mar 24, 2021 · 9 comments · Fixed by #325 or #364

Comments

@jshughes
Copy link
Collaborator

LDDTool needs to allow class/attributes defined using DD_Associate_External_Class so that they can be inherited. The class and attribute defined using DD_Associate_External_Class must be defined within context (e.g., xpath) in order to be inherited by one or more sub classes.

Motivation

This is an additional feature that will support the partitioning of the Geometry Dictionary

jshughes added a commit that referenced this issue Mar 24, 2021
…D_Associate_External_Class

LDDTool needs to allow class/attributes defined using DD_Associate_External_Class to be inherited. The class and attribute defined using DD_Associate_External_Class must be defined within context (e.g., xpath) in order to be inherited by one or more sub classes.

Resolves #324
jshughes added a commit that referenced this issue Mar 24, 2021
…D_Associate_External_Class (#325)

LDDTool needs to allow class/attributes defined using DD_Associate_External_Class to be inherited. The class and attribute defined using DD_Associate_External_Class must be defined within context (e.g., xpath) in order to be inherited by one or more sub classes.

Resolves #324
@jshughes jshughes reopened this Jun 3, 2021
@jshughes
Copy link
Collaborator Author

jshughes commented Jun 3, 2021

Fix Errors:

    • Class:0001_NASA_PDS_1.geom.Geometry Association:geom.Geometry_Orbiter Class:geom.Geometry_Orbiter - Missing Component Class
    • ERROR Association: geom.Geometry_Orbiter - Missing Component - Reference Type: component_of
    • ERROR Class: - At least one class must be defined as an xs:Element. (<element_flag> set to "true")

@jshughes jshughes closed this as completed Jun 3, 2021
jshughes added a commit that referenced this issue Jun 3, 2021
…lement_flag> error

LDDTool Geometry Partitioning Task - Remove the reported error "At least one class must be defined as an xs:Element. (<element_flag> set to true)". Since the dictionary is partioned, only one partition is required to have an <element_flag> set to "true".

Resolves #324
jshughes added a commit that referenced this issue Jun 3, 2021
…lement_flag> error (#364)

LDDTool Geometry Partitioning Task - Remove the reported error "At least one class must be defined as an xs:Element. (<element_flag> set to true)". Since the dictionary is partioned, only one partition is required to have an <element_flag> set to "true".

Resolves #324
@rchenatjpl
Copy link

rchenatjpl commented Jun 5, 2021

@jshughes @jordanpadams
I didn't see a sample Ingest_LDD for this, so I found and butchered one and noticed a feature which could be fine or not. The attached Ingest_LDD has (heavily chopped):

    <DD_Class>
        <name>Measurement_Instrument</name>
        <DD_Associate_External_Class>
            <namespace_id>pds</namespace_id>
            <class_name>Internal_Reference</class_name>
            <DD_Context_Value_List>
                <attribute_name>reference_type</attribute_name>

which results in the .xsd having

  <xs:complexType name="Measurement_Instrument">
    <xs:sequence>
      <xs:element name="Measurement_Instrument_Internal_Reference" type="test:Measurement_Instrument_Internal_Reference"> </xs:element>
    </xs:sequence>
  </xs:complexType>
  <xs:complexType name="Measurement_Instrument_Internal_Reference">
    <xs:sequence>
      <xs:element name="Measurement_Instrument_reference_type" type="test:Measurement_Instrument_reference_type"> </xs:element>
    </xs:sequence>
  </xs:complexType>

My question: do we care that pds:Internal_Reference has 2 required elements: lid_reference and reference_type? The Ingest_LDD only has the latter, so the output .xsd does also, so no lid_reference.

@rchenatjpl
Copy link

Archive.zip

@jordanpadams
Copy link
Member

@rchenatjpl doesn't internal reference require lid_reference all the time? I think the point of this new class is to extend permissible values for an attribute, but not change the definition of the class and it's associated attributes. Are you seeing some other unexpected behavior? @jshughes does this sound right?

@rchenatjpl
Copy link

rchenatjpl commented Jun 8, 2021

I guess the requester wants lddtool to do something else, but I'm unsure.
Here's another perspective. For the Ingest_LDD in the earlier Archive.zip,
lddtool produces a dictionary that allows a minimal data label (im324.xml):

  <test:Measurement_Instrument>
    <test:instrument_name>1.1</test:instrument_name>
    <test:Measurement_Instrument_Internal_Reference>
      <test:Measurement_Instrument_reference_type>measurement_parameters_to_instrument</test:Measurement_Instrument_reference_type>
    </test:Measurement_Instrument_Internal_Reference>
  </test:Measurement_Instrument>

I hand-edited the dictionary to allow a minimal data label (im324b.xml):

  <test:Measurement_Instrument>
    <test:instrument_name>1.1</test:instrument_name>
    <test:Internal_Reference>
      <lid_reference>urn:a:b:c</lid_reference>
      <reference_type>measurement_parameters_to_instrument</reference_type>
    </test:Internal_Reference>
  </test:Measurement_Instrument>

GOOD: reference_type is now mandatory
GOOD: no prepended "Measurement_Instrument_", e.g. "Internal_Reference"
is a better name than "Measurement_Instrument_Internal_Reference"
BAD: maybe must use test:Internal_Reference, not pds:Internal_Reference
BAD: is it possible for lddtool to create such a .xsd and .sch?

The hand-edited .sch and .xsd and the new minimal data label are in the
attached ArchiveB.zip. The 4th file im324.tab is unchanged.

Richard
ArchiveB.zip

@jshughes
Copy link
Collaborator Author

jshughes commented Jun 8, 2021

There are about three related issues regarding DD_Associate_External_Class. I am working a couple that Anne Raugh found associated with the Geometry LDD partitioning task. I am attaching the two Geom LDDs. I believe that Richard will also find the issue he found here. Richard, the classes are Body_Identification_Base and Frame_Identification_Base, both defined in GeomOrbit.
GeomPart.zip

@jordanpadams
Copy link
Member

@jshughes is this still and issue or has this been closed by some other bug fix / update?

@jshughes
Copy link
Collaborator Author

This issue can be closed since the original use cases involving DD_Associate_External_Class have been addressed. The results have been tested and accepted by the original SCR reporter.

Background: Referring to the original issue pasted in below, the implication is that the class and attribute are being redefined. This is not exactly the case. In the current implementation the local namespace references the external class/attribute definition directly (e.g., pds:Internal_Reference - from the Common dictionary) and then "restricts" the attribute definition by defining new standard values in the local namespace. The resulting XML Schema references the external attribute using "ref:" and the local standard values are validated using Schematron rules.

This means that the issues associated with local attribute redefinitions, e.g., "Measurement_Instrument_reference_type" have been resolved. These "redefinitions" are no longer created and the local references now refer to the definitions of the attribute in the external namespace, e.g., pds:reference_type and pds: Internal_Reference.

Note: The issue of whether the attribute is actually redefined might arise in the future since currently there are DDWG discussions on the types of cross-referencing that should be allowed.

---Original_Issue---
LDDTool needs to allow class/attributes defined using DD_Associate_External_Class so that they can be inherited. The class and attribute defined using DD_Associate_External_Class must be defined within context (e.g., xpath) in order to be inherited by one or more sub classes.

@jordanpadams
Copy link
Member

thanks for the details @jshughes ! that is more than enough to close this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment