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.

Sign up for GitHub

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

add missing AOM 2 fields and types #2

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

add missing AOM 2 fields and types #2

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0fe2c5f
Add attribute tuples to AOM 2 XSD
pieterbos Feb 14, 2019
697efce
Add missing field to terminology
pieterbos Feb 14, 2019
f5a9386
Add TEMPLATE and TEMPLATE_OVERLAY
pieterbos Feb 14, 2019
31aa0b7
Add operational_template
pieterbos Feb 14, 2019
e52eaf8
Correct indentation
pieterbos Feb 14, 2019
128f413
Apply two suggestions from code review
sebastian-iancu Feb 26, 2019
6311ea5
Fixes to the AOM XSD after review and discussion
pieterbos Feb 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 75 additions & 2 deletions components/AM/latest/Archetype.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"
xmlns="http://schemas.openehr.org/v2" targetNamespace="http://schemas.openehr.org/v2" elementFormDefault="qualified"
id="Archetype.xsd" version="v2.0.6"
xmlns:vc="http://www.w3.org/2007/XMLSchema-versioning" vc:minVersion="1.1">
xmlns:vc="http://www.w3.org/2007/XMLSchema-versioning" vc:minVersion="1.1">
<!-- dependencies -->
<xs:include schemaLocation="Rules.xsd"/>
<xs:include schemaLocation="ArchetypeCommon.xsd"/>
Expand Down Expand Up @@ -56,6 +56,39 @@
</xs:extension>
</xs:complexContent>
</xs:complexType>

<xs:complexType name="TEMPLATE">
<xs:complexContent>
<xs:extension base="AUTHORED_ARCHETYPE">
<xs:sequence>
<xs:element name="template_overlays" type="TEMPLATE_OVERLAY" nillable="true" minOccurs="0" maxOccurs="unbounded"/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is @nillable necessary here (and also on next OPERATIONAL_TEMPLATE)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you're right. No, it shouldn't be allowed, I'll change it soon.

</xs:sequence>
</xs:extension>
</xs:complexContent>
</xs:complexType>

<xs:complexType name="TEMPLATE_OVERLAY">
<xs:complexContent>
<xs:extension base="ARCHETYPE">
<xs:sequence/>
</xs:extension>
</xs:complexContent>
</xs:complexType>

<xs:complexType name="OPERATIONAL_TEMPLATE">
<xs:complexContent>
<xs:extension base="AUTHORED_ARCHETYPE">
<xs:sequence>
<xs:element name="component_terminologies" type="INCLUDED_TERMINOLOGY" nillable="true" minOccurs="0" maxOccurs="unbounded"/>
<xs:element name="terminology_extracts" type="INCLUDED_TERMINOLOGY" nillable="true" minOccurs="0" maxOccurs="unbounded"/>
</xs:sequence>
</xs:extension>
</xs:complexContent>
</xs:complexType>

<xs:complexType name="INCLUDED_TERMINOLOGY">
</xs:complexType>

<xs:complexType name="ARCHETYPE_CONSTRAINT" abstract="true">
<xs:sequence/>
</xs:complexType>
Expand Down Expand Up @@ -98,6 +131,7 @@
<xs:extension base="C_DEFINED_OBJECT">
<xs:sequence>
<xs:element name="attributes" type="C_ATTRIBUTE" minOccurs="0" maxOccurs="unbounded"/>
<xs:element name="attribute_tuples" type="C_ATTRIBUTE_TUPLE" minOccurs="0" maxOccurs="unbounded"/>
</xs:sequence>
</xs:extension>
</xs:complexContent>
Expand Down Expand Up @@ -147,6 +181,37 @@
</xs:complexContent>
</xs:complexType>

<xs:complexType name="C_SECOND_ORDER" abstract="true">
pieterbos marked this conversation as resolved.
Show resolved Hide resolved
<!-- this should have "members", but then it would need to be further restricted by descendants, plus extended. -->
</xs:complexType>
<xs:complexContent>
<!-- this should have members, but then it would need to be further restricted by descendants, plus extended.
Not sure how to do that in XSD :) -->
<xs:sequence/>
</xs:complexContent>
</xs:complexType>

<xs:complexType name="C_ATTRIBUTE_TUPLE">
<xs:complexContent>
<xs:extension base="C_SECOND_ORDER">
<xs:sequence>
<xs:element name="members" type="C_ATTRIBUTE" minOccurs="0" maxOccurs="unbounded"/>
<xs:element name="tuples" type="C_PRIMITIVE_TUPLE" minOccurs="0" maxOccurs="unbounded"/>
</xs:sequence>
</xs:extension>
</xs:complexContent>
</xs:complexType>

<xs:complexType name="C_PRIMITIVE_TUPLE">
<xs:complexContent>
<xs:extension base="C_SECOND_ORDER">
<xs:sequence>
<xs:element name="members" type="C_PRIMITIVE_OBJECT" maxOccurs="unbounded"/>
</xs:sequence>
</xs:extension>
</xs:complexContent>
</xs:complexType>

<xs:complexType name="C_BOOLEAN">
<xs:complexContent>
<xs:extension base="C_PRIMITIVE_OBJECT">
Expand Down Expand Up @@ -248,10 +313,18 @@
</xs:complexType>
<xs:complexType name="ARCHETYPE_TERMINOLOGY">
<xs:sequence>
<!-- possible here, but redundant: is_differential, original_language, concept_code. add? -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add them as optional

<xs:element name="term_definitions" type="CodeDefinitionSet" maxOccurs="unbounded"/>
<xs:element name="term_bindings" type="TermBindingSet" minOccurs="0" maxOccurs="unbounded"/>
<xs:element name="value_sets" type="VALUE_SET" minOccurs="0" maxOccurs="unbounded"/>
<xs:element name="terminology_extracts" type="CodeDefinitionSet" minOccurs="0" maxOccurs="unbounded"/>
</xs:sequence>
</xs:complexType>
<xs:complexType name="INCLUDED_TERMINOLOGY">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this originating from? I cannot find the specs for this; I assume is related to OPERATIONAL_TEMPLATE, but so far there I see no specs clearly specifying it.
If this is not used yet (so this is a new class introduced by this change), I suggest to name it ArchetypeTerminologyItem to be in line with other similarly defined classes.

Suggested change
<xs:complexType name="INCLUDED_TERMINOLOGY">
<xs:complexType name="ArchetypeTerminologyItem">
<xs:complexContent>
<xs:extension base="ARCHETYPE_TERMINOLOGY">
<xs:attribute name="id" type="xs:string" use="required"/>
</xs:extension>
</xs:complexContent>
</xs:complexType>

Copy link
Author

@pieterbos pieterbos Feb 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Operational template has the component_terminogies field/element. that is where this is used. It is defined in https://specifications.openehr.org/releases/AM/latest/AOM2.html#_operational_template_class as:

Compendium of flattened terminologies of archetypes externally referenced from this archetype, keyed by archetype identifier. This will almost always be present in a[n operational] template.

So it is literally a complete included terminology, plus the archetype id where it is from. That's why I called it INCLUDED_TERMINOLOGY, but indeed this is just an XML complexType, not an OpenEHR type. I could rename it. Given this information, do you think it should be IncludedTerminology or ArchetypeTerminologyItem?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reasoning was that this class (type) is also used at OPERATIONAL_TEMPLATE.terminology_extracts, therefore has to be named differently than 'includedTerminology'. It should also be descriptive, i.e. should indicate that this is a type used as one xml-item of the Hash<String,ARCHETYPE_TERMINOLOGY>.
Perhaps @wolandscat have a better suggestion?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you are proposing two things:

  • extend ARCHETYPE_TERMINOLOGY instead of including it as a parameter.
  • find a better name

extending: sounds good, I'll change that.

The name:
This really is another terminology, or an extract of it which itself is a terminology as well, included in this archetype. Both for the component_terminologies and terminology_extracts. So I think INCLUDED_TERMINOLOGY is a fitting name. It could be IncludedTerminology as well of course.
It also is an archetype terminology keyed by its id-field. That could be: ArchetypeTerminologyDictionaryItem , just like StringDictionaryItem. Perhaps we need a native speaker. @wolandscat ?

Actually the terminology_extracts attribute on OPERATIONAL_TEMPLATE does not make sense to me at all on the operational template level - it is keyed by value/at-code, but an at-code in an OPT is meaningless without its path, since it can come from one of many included archetypes. I would say the whole attribute needs to be removed, and the already existing terminology_extracts in the component_terminologies should be used instead. But that is a different issue, I'll create a problem report.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On reading the above.... I'd probably go for IncludedTerminology, so it doesn't make it look like there is an AOM type of that name (there isn't). But 'included terminology' is more informative than 'ArchetypeTerminologyItem' which doesn't tell you anything much, even if it is somehow more technically 'right'.

I don't tend to get too religious about what appears in an XSD, except for one thing - it's good practice to make it clear which classes are direct reprsentations of a class in the upstream specs (RM, AM etc) and which ones are not (can be quite a few helper classes in an XSD). I also think it's better to make the XSDs as intelligible as possible in a standalone way for implementers who are not going to go look in the upstream specs. Just my view ;)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed. There are many more issues probably in these files. I'll generate a schema from archie, and see how that looks.

<xs:sequence>
<xs:element name="items" type="ARCHETYPE_TERMINOLOGY"/>
</xs:sequence>
<xs:attribute name="id" type="xs:string" use="required"/>
</xs:complexType>

</xs:schema>
</xs:schema>