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

Implement FHIRPath select() function #9

Closed
wants to merge 1 commit into from
Closed

Conversation

Quarz0
Copy link

@Quarz0 Quarz0 commented Jun 28, 2024

Implement FHIRPath select() function

@Quarz0
Copy link
Author

Quarz0 commented Jun 28, 2024

This implementation for select() should return an error if an invalid field is used as an argument, which matches the current behaviour of where() and other functions/expressions.

However I wanted to note that the spec is a bit vague on this part, which is whether an invalid field or identifier should return an error or empty.
For example, in https://hl7.org/fhirpath/N1/#identifiers, it mentions that If the identifier cannot be resolved, the evaluation will end and signal an error to the calling environment.
While in https://hl7.org/fhirpath/N1/#repeatprojection-expression-collection, there's the following example expression: Questionnaire.descendants().select(item). If invalid fields should return an error, then this example shouldn't be valid since not all nodes will have the field item. But if returns empty, then this expression should be valid.

Looking also at other FHIRPath implementations in https://fhirpath-lab.com/FhirPath, some of them return an error, and some return empty.

@Quarz0
Copy link
Author

Quarz0 commented Jun 28, 2024

Looking a bit closer, it seems that some of the implementations that return an error would actually accept the expressionQuestionnaire.descendants().select(item). The logic in this case seems to be if that field exists in any of the input nodes, then no error is returned (even if the field is not set).

But the logic seems a bit confusing since for the nodes that don't have this field, we still need to evaluate it instead of returning an error. Example expression

Anyway, this is not specific to select() so if we want to use this behaviour, it can be done in a separate PR

@Quarz0 Quarz0 changed the title Main Implement FHIRPath select() function Jun 29, 2024
@VickSuresh
Copy link
Contributor

@Quarz0 Thanks for the PR! Our implementation was created with https://hl7.org/fhirpath/N1/#identifiers in mind, so we raise an error for any invalid identifiers. It looks like the firely implementation checks each item in the input collection, and if there is a valid field for at least one, no error is raised. See example

@Quarz0
Copy link
Author

Quarz0 commented Jul 2, 2024

Yeah, but I feel like not raising an error if at least one field is valid might make sense given the example you provided and the one in the specs and that almost all other implementations I could find don't return an error in this case

Also in the official fhir-test-cases repo, you can find the test case: Questionnaire.descendants().linkId.distinct().count() which also seems to indicate that no error should be reported if the field is valid for at least one of the inputs

@VickSuresh
Copy link
Contributor

Agreed. I'll reconcile this and merge through our internal process!

@VickSuresh VickSuresh closed this Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants