tracefirst / usaha_committee

XML schema for electronic CVIs
8 stars 2 forks source link

Test Result Value Type? #3

Closed mkm1879 closed 8 years ago

mkm1879 commented 11 years ago

The test result uses xs:all to combine the various potential types (integer, string, and float) of result data. This allows any combination of of the types zero or one of each in any order. Is that what we want? I assume this is to allow the kinds of things like a value (numeric) plus units (text) or interpretation (text). Or do we really expect the behavior of xs:choice? To completely model the universe of possible result formats is complicated. It is an entire data model in HL7. The existing solution may, in fact be what we want, but I would like everyone to see how this maps to their implementations.

mmcgrath commented 11 years ago

Good catch Mike -- as it happens, the software we have that generate XML docs only ever generates a single result so this is an unintended configuration in our schema. My vote would be to replace xs:all with xs:choice so that a single result is catered for.

My rationale is that this will make it simpler for people implementing systems to handle documents that bind against this schema...

Can we get thoughts on alternative approaches including leaving the schema as-is?

mmcgrath commented 11 years ago

7/16 call -- MM expressed preference for xs:choice. MMcG agrees. Leave open until next call to get more feedback...

mmcgrath commented 11 years ago

This has been open for four weeks without further comment -- I will make the "xs:choice" amendment as proposed by @mkm1879 and seconded by me.

sglCO commented 11 years ago

Sorry for missing this. You've already made the change but I figured I would add that I'm in agreement with xs:choice over xs:all for simplicity sake.

mkm1879 commented 11 years ago

We have been talking at cross purposes here for some time. Getting the definition of ResultValueType confused with the definition of TestType. Each test has a single Result element and some attributes. The Result element is a series of one or more elements of the three names/types listed. What xs:all did was allow for one result to have more than one part. So, you could have a Ct value (ResultFloat) and an interpretation (ResultString). Of course 99.9% of the time you will have just one in which case there is no problem.

IMHO the change from xs:sequence to xs:choice in TestType was unnecessary and probably slightly confusing. I now think my original suggestion to change xs:all to xs:choice was wrong. I didn't understand the use case I've tried to explain here.

sglCO commented 11 years ago

I may be a bit confused myself but from your clarification it sounds like the use case requires the ability to send a result interp as well a CT value thus the need for xs:all and not xs:choice. If I'm following correctly, I see why you're saying the change is wrong now. -Shane

mkm1879 commented 11 years ago

Yes, my use case argues against my original concern. The xs:all really was the right solution. I just didn't see that before.

mmcgrath commented 11 years ago

I see a "git revert" on the horizon ;-)

I want to run a few sample scenarios -- I find looking at a sample XML fragment (that binds to the schema) much more enlightening than looking at the schema itself....

mmcgrath commented 11 years ago

Here's an example:

<Animal Age="2M" Breed="HOL" Sex="Female">
    <AnimalTag Number="VTF005788" Type="OTH"/>
    <AnimalTag Number="4104" Type="MGT"/>
    <Test idref="A1" TestCode="BRUC" ResultName="RESULT">
        <Result><ResultString>NEG</ResultString></Result>
    </Test>
    <Test idref="A1" TestCode="BRUC" ResultName="RESULT">
        <Result><ResultFloat>0.0</ResultFloat></Result>
    </Test>
    <Test idref="A1" TestCode="BRUC" ResultName="COMMENT">
        <Result><ResultString>Performed by MMcGrath</ResultString></Result>
    </Test>
</Animal>

I don't think either approach is wrong -- it just forces you to describe the results differently.....

sglCO commented 11 years ago

I see what you did there...good point :). Interested to see opinions from other members whether a particular approach might be preferred, at least with respect to simplicity.

mkm1879 commented 11 years ago

So for this, you would NOT need the xs:all because you have only one ResultType per Result. But you are repeating Test unnecessarily.

The ResultType definition seems to add an extra layer but I see that is the only way to make each of the result values strongly typed. That is a good thing, I think.

This XML:

NEG 0.0 Performed by MMcGrath

Validates with this definition of TestType:

<xs:complexType name="TestType">
    <xs:sequence>
        <xs:element name="Result" type="ResultValueType" maxOccurs="unbounded"/>
    </xs:sequence>
    <xs:attribute name="idref" type="xs:IDREF" use="required">
        <xs:annotation>
            <xs:documentation>This *must* be one of the ID values in the Accessions included with this document. If any Animal Test references an Accession ID that doesn't exist, the document will not be valid.</xs:documentation>
        </xs:annotation>
    </xs:attribute>
    <xs:attribute name="TestCode" type="xs:string" use="required"/>
</xs:complexType>
<xs:simpleType name="ResultNameType">
    <xs:annotation>
        <xs:documentation>This is the result type which can either be RESULT or COMMENT</xs:documentation>
    </xs:annotation>
    <xs:restriction base="xs:string">
        <xs:enumeration value="RESULT"/>
        <xs:enumeration value="COMMENT"/>
    </xs:restriction>
</xs:simpleType>
<xs:complexType name="ResultValueType">
    <xs:annotation>
        <xs:documentation>You should choose the correct type for the result based on your source data type. We support Integer, Float and String.</xs:documentation>
    </xs:annotation>
    <xs:choice>
        <xs:element minOccurs="0" name="ResultInteger" type="xs:integer"/>
        <xs:element minOccurs="0" name="ResultString" type="xs:string"/>
        <xs:element minOccurs="0" name="ResultFloat" type="xs:float"/>
    </xs:choice>
    <xs:attribute name="ResultName" type="ResultNameType"/>
</xs:complexType>

What do you think?

Mike

From: mmcgrath [mailto:notifications@github.com] Sent: Thursday, August 22, 2013 11:19 AM To: tracefirst/usaha_committee Cc: Michael Martin Subject: Re: [usaha_committee] Test Result Value Type? (#3)

Here's an example:

``` NEG 0.0 Performed by MMcGrath ```

I don't think either approach is wrong -- it just forces you to describe the results differently.....

— Reply to this email directly or view it on GitHubhttps://github.com/tracefirst/usaha_committee/issues/3#issuecomment-23098143.

mmcgrath commented 11 years ago

@mkm1879 I am happy with your design/structure with one exception -- I think that the attribute in the second-last line (i.e. the ResultName) should be required. I'll do a commit to incorporate your changes with the ResultName being required and we can further discuss if required...

Thanks Michael

sglCO commented 11 years ago

Mike(s), this structure looks good! I'd also agree with making ResultName required to prevent a test element from being sent empty. Although, that did get me wondering how we would handle cases where no result was obtained (i.e. no test was performed due to bad specimen, etc). Perhaps we can discuss that situation further unless I'm missing something obvious which is entirely possible. - Shane

mmcgrath commented 10 years ago

There's been no objection recorded on this issue since I last amended the Test recording structure. I'll close this issue on 29 Nov unless there are objections before then