FHIR Chat · Snapshot reconciliation

Stream: conformance

Topic: Snapshot reconciliation

Grahame Grieve (Sep 25 2019 at 04:30):

Hey @Michel Rutten I'm looking at your comments on the snapshot generation tests.

Grahame Grieve (Sep 25 2019 at 04:30):

===== T4a =====

ok fixed.

Grahame Grieve (Sep 25 2019 at 04:31):

Error: element Patient.extension should inherit constraints from DomainResource.extension

well, see this in the java code:

Grahame Grieve (Sep 25 2019 at 04:31):

  private boolean checkExtensionDoco(ElementDefinition base) {
    // see task GF#3970. For an extension, there's no point copying across all the underlying definitional stuff
    boolean isExtension = base.getPath().equals("Extension") || base.getPath().endsWith(".extension") || base.getPath().endsWith(".modifierExtension");
    if (isExtension) {
      base.setDefinition("An Extension");
      base.setShort("Extension");
      base.setCommentElement(null);
      base.setRequirementsElement(null);
      base.getAlias().clear();
      base.getMapping().clear();
    }
    return isExtension;
  }

Grahame Grieve (Sep 25 2019 at 04:33):

though that doesn't explain why the constraints are missing.

Grahame Grieve (Sep 25 2019 at 04:39):

and that is because I don't replicate ele-1 and ext-1 everywhere in the underlying definitions. I don't believe I've seen anything in the technical correction task list to change that either

Grahame Grieve (Sep 25 2019 at 04:39):

@Ewout Kramer @Lloyd McKenzie etc - have we ever talked about this?

Lloyd McKenzie (Sep 25 2019 at 05:26):

Not that I recall. I think the intention was to not spit out tons of useless verbiage, not to avoid inheriting constraints

Grahame Grieve (Sep 25 2019 at 07:19):

I was more asking about ele-1 and ext-1 not appearing everywhere in the base package

Ewout Kramer (Sep 25 2019 at 09:42):

We haven't talked about them, since why would they need special treatment? When you don't copy these into each element, how would a generic validator know to run these invariants? If we don't want them everywhere because it takes up too much space, we need to document them as special exceptions so validators could hard-wire their validation.

Grahame Grieve (Sep 25 2019 at 10:23):

Well, I did do that but I didn’t really think about. I wrote a big long explanation about why we shouldn’t reproduce this in the snapshot but of course, that’s what snapshots should do. But does that mean you’re running off a jacked base Package? What other differences are there?

Ewout Kramer (Sep 25 2019 at 12:30):

Sort of, we're using the base package, but our tools that need the snapshot (like the validator) will regenerate the snapshot before it runs. Simplifier also recreates the snapshot based on the packages available in its database. So, the differential is king for us - the snapshot is something we compute ourselves.

Ewout Kramer (Sep 25 2019 at 12:31):

Other differences.....the very useful exercise you and @Michel Rutten are now doing should surface those, I hope. Or maybe you mean other kinds of differences?

Grahame Grieve (Sep 25 2019 at 12:44):

no I came to the same conclusions... I don't regenerate base snapshots, but I will be regenerating snapshots for derived packages in the IG Publisher from now on (found bugs due to bad input snapshots yesterday)

Grahame Grieve (Sep 25 2019 at 12:45):

but this process will surface whatever

Michel Rutten (Sep 25 2019 at 14:47):

FYI The SnapshotGenerator class in the .NET API exposes a configuration setting SnapshotGeneratorSettings.ForceRegenerateSnapshots. Forge and Simplifier enable this setting. This forces re-generation of all externally referenced snapshots, including core resource & datatype profiles.

Michel Rutten (Sep 25 2019 at 14:48):

https://github.com/FirelyTeam/fhir-net-api/blob/462d7a0cfe45a2faaac85e6aa874c1b8f23999d9/src/Hl7.Fhir.Specification/Specification/Snapshot/SnapshotGeneratorSettings.cs#L65

Grahame Grieve (Sep 27 2019 at 07:10):

ok, well, @Michel Rutten I updated the generated snapshots in the current build. but I also found out how I was doing things.. the validator was validating by the element, then by the type. Now I have to change the validator not to raise repeated errors from the same invariant which is on both the element and the type

Grahame Grieve (Sep 27 2019 at 07:10):

anyway, can you check the generated snapshots in the current build

Grahame Grieve (Sep 29 2019 at 08:10):

@Michel Rutten changing the snapshot generation for your comments on T11 has completely changed all the other tests?

Grahame Grieve (Sep 29 2019 at 08:12):

e.g. T1 forward...

Grahame Grieve (Sep 30 2019 at 05:02):

T23 - yes, deliberately out of order; the generated snapshot has them in the correct order. Note sort=true in the manifest

Michel Rutten (Sep 30 2019 at 10:00):

changing the snapshot generation for your comments on T11 has completely changed all the other tests?

Yes, correcting the base of Extension will affect many snapshots, as expected. However this change shouldn't break any snapshot manifest tests - unless they specifically inspect (inherited) properties of extension elements below the root.

Michel Rutten (Sep 30 2019 at 10:03):

T23 - yes, deliberately out of order

Indeed, initially I didn't notice that this test is specifically to verify behavior for invalid element order.
The .NET Implementation is not (yet) capable of rectifying element order, although I would like to add such logic eventually.

Grahame Grieve (Sep 30 2019 at 10:45):

ok well, you could look at the updated tests then, though you can't pass the unless I give you updated definitions for the forthcoming technical correction