Issue 3: Support Broadcasting the FHIR ValueSet #5
Conversation
bdrillard
force-pushed
the
BUNSEN-3-3
branch
2 times, most recently
from
ea3998a to
9402d54
Compare
| } | ||
|
|
||
| @Test | ||
| public void testValuesAreSorted() { |
There was a problem hiding this comment.
Here I was wanting to enforce the same sorting behavior that I thought was described in ConceptMapsTests, but that I noticed the test didn't fully describe sorting behavior since it lacks an additional group to sort against. It would actually appear that since groups are object-copied when removing the concepts, whatever order groups were specified is kept, even if that order isn't lexicographically ascending.
If keeping a sorting contract isn't necessary, we can remove the tests that would check it from both ConceptMapsTests and here.
bdrillard
force-pushed
the
BUNSEN-3-3
branch
3 times, most recently
from
5dde7a1 to
7b9edda
Compare
bdrillard
force-pushed
the
BUNSEN-3-3
branch
2 times, most recently
from
07e725e to
f8feea0
Compare
|
|
||
| /** | ||
| * @author AE032532 | ||
| */ |
There was a problem hiding this comment.
Looks like an unnecessary author tag was generated by an IDE here.
|
The build failed on CheckStyle warnings. (For future reference, running mvn clean install locally will expose these warnings.) |
|
Changes look good at first glance. I'll have a chance to do a more in depth review later this morning or early afternoon. |
bdrillard
force-pushed
the
BUNSEN-3-3
branch
2 times, most recently
from
3222a0e to
533d79e
Compare
Issue 3: finishing valuesets api and tests Issue 3: additional doc Issue 3: running broadcastablevaluesets tests Issue 3: changes to ConceptMaps Issue 3: updating tests for valuesetudfs Issue 3: tests working but valuesetudfs Issue 3: adding new values implementation Issue 3: finishing changes Issue 3: initial python updates, existing tests pass Issue 3: supporting arbitrary value sets Issue 3: adding tests over value set udfs Issue 3: using datasets for operations Issue 3: solving issue with large descendant systems Issue 3: using Hierarchies for snomed and loinc Issue 3: updating python tests for hierarchies Issue 3: updating python doc
|
I noticed we aren't exposing |
|
+1 to merge. I verified this build works with full LOINC and SNOMED and looks good. I think there are additional tweaks we might make to the Python API, but I'd rather break those up into a separate, focused PR. This one already has quite a lot of updates and is generally solid, so I think we can land this and iterate afterwards. |
|
Merged. Thanks for the review! |
Summary
This PR provides the enhancement described in #3, adding support for FHIR ValueSets and changing the underlying implementation of the
ValueSetUdfsclass to utilize ValueSets and a new Hierarchies class rather than ConceptMaps.Additionally, it enforces immutable semantics across all tables holding data for ValueSets, ConceptMaps, and Hierarchies by ensuring the Url and Versions for the resources are unique.
It also makes minor changes to normalize style/doc where inconsistencies/misspellings were encountered.
Additional Details
This PR adds a full test suite for ValueSets and Hierarchies, and additional test cases in the Python API over ValueSet functions and ValueSetUdf functions.