FHIR Chat · Type[x], Slices, open/closed · conformance

Stream: conformance

Topic: Type[x], Slices, open/closed


view this post on Zulip Grahame Grieve (Nov 30 2019 at 20:03):

I'm continuing to work through the issues the snapshot generator has with the Dutch profiles.

I've run into something I'm not sure we've discussed, around slicing on elements that have a choice of types.

In (B) the base spec, an element that has a choice of types is sliced by type on $this, and closed.
In (C) a derived profile, the profile constrains one of the types, but says that the slicing is open
In (D) a derived profile on profile (C), it profiles one of the types (a different one) but says that the slicing is open
In (E) a derived profile on profile (C), it profiles one of the types (the same one) but says that the slicing is open

.. the java snapshot generator just refuses to process this.

I think that it remains closed, irrespective of what C|D|E claim... but they just don't close off the other slices if they claim it's open. Is that right?

I'm hoping someone from firely (@Michel Rutten, @Martijn Harthoorn, @Christiaan Knaap ?) can clarify how the C# snapshot generator works on this

view this post on Zulip Christiaan Knaap (Dec 02 2019 at 13:29):

@Marco Visser for the C# snapshot generator

view this post on Zulip Grahame Grieve (Dec 03 2019 at 01:19):

I added some tests around this. I strongly encourage @Lloyd McKenzie and @Marco Visser to review this commit:

view this post on Zulip Grahame Grieve (Dec 03 2019 at 01:20):

https://github.com/FHIR/fhir-test-cases/commit/81ae5978bc2b8562e5aff0da066a0d80d453bea8

view this post on Zulip Marco Visser (Dec 03 2019 at 09:39):

Thanks @Grahame Grieve I will incorporate those tests in our fhir-net-api unittests in the coming week. But first I am all focused on the technical correction (4.0.1), so that our fhir-net-api is working again.

view this post on Zulip Grahame Grieve (Dec 03 2019 at 09:42):

ok thanks

view this post on Zulip Marten Smits (Dec 06 2019 at 12:22):

Hi Grahame, I'm looking into this now. Got them loaded into our unit tests. First issue I encountered was that obs 1-0 obs1-1 and obs 1-2 all have to same canonical URL. And that obs1-1 and obs 1-2 have a baseDefinition that is the same as their canonical URLs.

And obs1_2, obs2_3, and obs3_0, don't have an -expected file. Is that on purpose?

view this post on Zulip Grahame Grieve (Dec 06 2019 at 18:20):

generating a snapshot fails for 1-2, 2-3 and 3 all are illegal so there's no snapshot

view this post on Zulip Grahame Grieve (Dec 06 2019 at 18:23):

I'll fix the URLs

view this post on Zulip Marten Smits (Dec 09 2019 at 09:32):

Alright, I'll compare our results, and will get back to you

view this post on Zulip Marten Smits (Dec 09 2019 at 15:24):

Compared results and have two major differences:
- In obs-1 you leave out the discriminator in the diff, and implicitly assume a slice on value[x] is a type slice, with discriminator $this. Is it best practice to leave it out of the diff? I'd expect an editor tool to handle implicit type slices and give the snapshot generator explicit instructions. The .NET snapshot generator in this case only "copies" over slicing.rules from the diff to the snapshot, and doesn't do the magic you do.
And íf you leave out the discriminator, you should add a slicing.description according to eld-1.

- In obs-2 the diff says:

 <element>
      <path value="Observation.value[x]"/>
      <slicing>
        <rules value="open"/>
      </slicing>
      <type>
        <code value="CodeableConcept"/>
      </type>
    </element>
    <element>
      <path value="Observation.value[x]"/>
      <sliceName value="valueCodeableConcept"/>
      <type>
        <code value="CodeableConcept"/>
      </type>
      <binding>
        <strength value="required"/>
        <valueSet value="http://somewhere/something"/>
      </binding>
    </element>
  </differential>

And in the snapshot slice.rules is suddenly closed, and all original types are repeated again in the type array. I think that this indeed means the same thing, but it is not behavior I'd expect at all. I always thought that if you edit the type array in the diff, the whole list becomes the new truth, and that this is also the appropriate way to constrain types out for example. Otherwise you can only do that by introducing a type slice and close it explicitly?
I thought type slices are always closed and you explicitly specify which types are allowed in the type array in the diff. And if you want to include all the allowed types, you leave out the type array entirely in the diff.

view this post on Zulip Grahame Grieve (Dec 10 2019 at 05:23):

Is it best practice to leave it out of the diff?

no. but it's allowed

view this post on Zulip Grahame Grieve (Dec 10 2019 at 05:24):

with regard to 2... I asked... but if the list of slices is closed, and you constrain one and leave the others open... then it's still closed when you're done, you just left the others untouched

view this post on Zulip Grahame Grieve (Dec 10 2019 at 05:24):

so that's what the test is testing

view this post on Zulip Marten Smits (Dec 10 2019 at 10:21):

Is it best practice to leave it out of the diff?

no. but it's allowed

Ok, I'll file an issue here to handle that in the .NET snapshotgenerator

view this post on Zulip Marten Smits (Dec 10 2019 at 10:54):

with regard to 2... I asked... but if the list of slices is closed, and you constrain one and leave the others open... then it's still closed when you're done, you just left the others untouched

Agree.

view this post on Zulip Marten Smits (Dec 10 2019 at 11:02):

But then you should still mention all possible types in the diff of the slice intro. Otherwise how will you specify this that you type slice (open), only allow codeableconcept and quantity, and put a binding on codeableconcept. I think that should look like this:

<differential>
   <element>
    <path value="Observation.value[x]"/>
    <slicing>
      <rules value="open"/>
    </slicing>
    <type>
      <code value="CodeableConcept"/>
    </type>
    <type>
      <code value="Quantity"/>
    </type>
  </element>
  <element>
    <path value="Observation.value[x]"/>
    <sliceName value="valueCodeableConcept"/>
    <type>
      <code value="CodeableConcept"/>
    </type>
    <binding>
      <strength value="required"/>
      <valueSet value="http://somewhere/something"/>
    </binding>
  </element>
</differential>

And if that's true, obs-2 actually describes only codeableconcept is allowed, right?

view this post on Zulip Grahame Grieve (Dec 11 2019 at 00:22):

actually, you're right. I updated the tests and added some more

view this post on Zulip Marten Smits (Dec 11 2019 at 08:37):

Nice, I'll make time tomorrow to have a look

view this post on Zulip Marten Smits (Dec 16 2019 at 16:24):

Didn't have time to test this last week and now my holiday has started, so I postponed this to the new year. Will get back to you then.

view this post on Zulip Marten Smits (Jan 09 2020 at 10:39):

@Grahame Grieve
I have picked this up again.

Loaded the new tests. I still have doubts about type slices automatically closing:

In obs-2 the differential states in the slice intro that the slice is open, and after that a binding is set to valueCodeableConcept
Can you explain why this slice would be closed after snapshot generation? This means I can't introduce a new slice in a derived profile, if I for example want to put a binding on valueQuantity, right?

view this post on Zulip Marten Smits (Jan 16 2020 at 13:58):

@Grahame Grieve is this still on something you are looking into/ thinking about?

view this post on Zulip Grahame Grieve (Jan 30 2020 at 20:28):

sorry. finally getting to this @Marten Smits

view this post on Zulip Grahame Grieve (Jan 30 2020 at 20:30):

  • In the base Observation, the type slicing is closed

view this post on Zulip Grahame Grieve (Jan 30 2020 at 20:31):

  • In obs-2-input, the profile slices it, and says that it's open. That is, it is making rules about one of the slices, and not saying anything about the others

view this post on Zulip Grahame Grieve (Jan 30 2020 at 20:32):

  • so the generated snapshot notes those rules, but still has the same closed slicing as the base Observation

view this post on Zulip Grahame Grieve (Jan 30 2020 at 20:32):

  • you can profile a different slice in a derived profile

view this post on Zulip Marten Smits (Feb 06 2020 at 14:44):

@Grahame Grieve Can you explain why type slicing is closed in the base Observation in obs-2? Is that the default for type slicing? Obs-2 is a profile on the core Observation.

view this post on Zulip Grahame Grieve (Feb 06 2020 at 19:05):

type slices re always closed, because you can't add any more slices than exist to start wit h

view this post on Zulip Marten Smits (Feb 10 2020 at 10:12):

Ah yeah fair enough

view this post on Zulip Ward Weistra (Feb 24 2020 at 09:34):

Hi @Grahame Grieve, we were having an internal discussion whether you see the use of <rules value="open"/> in obs-2-input.xml as a user error that you're ignoring or correct way of saying "it is making rules about one of the slices, and not saying anything about the others".

view this post on Zulip Grahame Grieve (Feb 24 2020 at 12:00):

this:

a correct way of saying "it is making rules about one of the slices, and not saying anything about the others

view this post on Zulip Ward Weistra (Feb 24 2020 at 13:18):

OK, so instead of slicing rules saying something about the value (open/closed) you want to have in the resulting snapshot, it says something about how the diff content should be interpreted.

Is the following description then correct?
Assuming we have a closed base slice with four allowed slices and I'm describing constraints for only two of them in my profile:

  • If I set slicing rules value to open: The resulting snapshot will still allow the four slices, of which two with additional constraints
  • If I set slicing rules value to closed: The resulting snapshot will only allow the two described slices with the additional constraints

Assuming we have an open base slice with four allowed slices and I'm describing constraints for only two of them in my profile:

  • If I set slicing rules value to open: The resulting snapshot will still allow the four slices, of which two with additional constraints, plus additional slices
  • If I set slicing rules value to closed: The resulting snapshot will only allow the two described slices with the additional constraints

view this post on Zulip Grahame Grieve (Feb 24 2020 at 19:31):

yes I think so

view this post on Zulip Ewout Kramer (Feb 25 2020 at 10:56):

Mmmm....the thing that's a bit weird here is that the slice open/close indication turns from an aspect of the slice to an interpretation hint for the snapshot generator. I.e. logically, you could never turn a closed slice in the base into an open one, but we now still allow it here because it has morphed into an indication of how the differential is to be interpreted. That might be nit-picking, and in any case I believe this train has probably left the station ;-)

view this post on Zulip Ewout Kramer (Feb 25 2020 at 10:58):

Maybe, for completeness, we should then add the final open/closed state of the slice after executing this interpretation. I think this is meant to be:

  • When the base was closed - the slice remains closed (even if though have "open" in the differential)
  • When the base was open - with diff=open => the slice remains open. with diff=closed => the slice becomes closed.

view this post on Zulip Grahame Grieve (Feb 25 2020 at 11:16):

yes.

view this post on Zulip Grahame Grieve (Feb 25 2020 at 11:16):

I don't see how it can be any other way

view this post on Zulip Grahame Grieve (Feb 25 2020 at 11:16):

(and welcome back)

view this post on Zulip Chris Grenz (Mar 10 2020 at 22:06):

Seems a bizarre interpretation to me...if the base is closed, the differential can't be anything but closed. If the differential intends to limit the list to a smaller set of slices, it should max:0 the disallowed slices, not provide a new list. IMO, a diff of open over a base of closed is an error.

view this post on Zulip Chris Grenz (Mar 10 2020 at 22:06):

And yes, welcome back @Ewout Kramer !


Last updated: Apr 12 2022 at 19:14 UTC