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

Add more CI testing and fix R4 generation #50

Merged
merged 1 commit into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 41 additions & 2 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,47 @@ jobs:
run: |
python -m pip install --upgrade pip
pip install -r requirements.txt
# isodate is used by our sample templates and pytest is our runner of choice
pip install isodate pytest

- name: Test generation
- name: Cache R5 download
uses: actions/cache@v4
with:
path: downloads-r5
key: downloads-r5

- name: Generate R5
run: |
cp ./Default/settings.py .
sed -i "s|'../models|'models|g" settings.py
sed -i "s|'downloads'|'downloads-r5'|g" settings.py
sed -i "s|\(^specification_url = \)'.*'|\1'http://hl7.org/fhir/R5'|g" settings.py
rm -rf models
./generate.py
grep 'Generated from FHIR 5.0.0' models/account.py # sanity check

# FIXME: The R5 tests fail to pass currently (due to some wrong types and missing properties)
# See https://github.com/smart-on-fhir/fhir-parser/issues/51
# - name: Test R5
# run: |
# FHIR_UNITTEST_DATADIR=downloads-r5 pytest

- name: Cache R4 download
uses: actions/cache@v4
with:
path: downloads-r4
key: downloads-r4

- name: Generate R4
run: |
echo "from Default.settings import *" > settings.py
cp ./Default/settings.py .
sed -i "s|'../models|'models|g" settings.py
sed -i "s|'downloads'|'downloads-r4'|g" settings.py
sed -i "s|\(^specification_url = \)'.*'|\1'http://hl7.org/fhir/R4'|g" settings.py
rm -rf models
./generate.py
grep 'Generated from FHIR 4.0.1' models/account.py # sanity check

- name: Test R4
run: |
FHIR_UNITTEST_DATADIR=downloads-r4 pytest
2 changes: 2 additions & 0 deletions Default/mappings.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@
'>': 'gt',
'>=': 'gte',
'*': 'max',
'+': 'pos',
'-': 'neg',
}

# If you want to give specific names to enums based on their URI
Expand Down
2 changes: 1 addition & 1 deletion Default/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
write_unittests = True
tpl_unittest_source = 'template-unittest.py' # the template to use for unit test generation
tpl_unittest_target = '../models' # target directory to write the generated unit test files to
tpl_unittest_target_ptrn = '{}_tests.py' # target file name pattern for unit tests; the one placeholder (`{}`) will be the class name
tpl_unittest_target_ptrn = '{}_test.py' # target file name pattern for unit tests; the one placeholder (`{}`) will be the class name
unittest_copyfiles = [] # array of file names to copy to the test directory `tpl_unittest_target` (e.g. unit test base classes)

unittest_format_path_prepare = '{}' # used to format `path` before appending another path element - one placeholder for `path`
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ If you've come here because you want _Swift_ or _Python_ classes for FHIR data m
- [Swift-FHIR][] and [Swift-SMART][]
- Python [client-py][]

The `main` branch is currently capable of parsing _R5_.
The `main` branch is currently capable of parsing _R4_
and has preliminary support for _R5_.

This work is licensed under the [APACHE license][license].
FHIR® is the registered trademark of [HL7][] and is used with the permission of HL7.
Expand Down Expand Up @@ -75,7 +76,6 @@ If an element itself defines a class, e.g. `Patient.animal`, calling the instanc
The class of this property is derived from `element.type`, which is expected to only contain one entry, in this matter:

- If _type_ is `BackboneElement`, a class name is constructed from the parent element (in this case _Patient_) and the property name (in this case _animal_), camel-cased (in this case _PatientAnimal_).
- If _type_ is `*`, a class for all classes found in settings` `star_expand_types` is created
- Otherwise, the type is taken as-is (e.g. _CodeableConcept_) and mapped according to mappings' `classmap`, which is expected to be a valid FHIR class.

> TODO: should `http://hl7.org/fhir/StructureDefinition/structuredefinition-explicit-type-name` be respected?
Expand Down
2 changes: 1 addition & 1 deletion Sample/template-unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def test{{ class.name }}{{ loop.index }}(self):
def impl{{ class.name }}{{ loop.index }}(self, inst):
{%- for onetest in tcase.tests %}
{%- if "str" == onetest.klass.name %}
self.assertEqual(inst.{{ onetest.path }}, "{{ onetest.value|replace('"', '\\"') }}")
self.assertEqual(inst.{{ onetest.path }}, "{{ onetest.value|replace('\\', '\\\\')|replace('"', '\\"') }}")
{%- else %}{% if "int" == onetest.klass.name or "float" == onetest.klass.name or "NSDecimalNumber" == onetest.klass.name %}
self.assertEqual(inst.{{ onetest.path }}, {{ onetest.value }})
{%- else %}{% if "bool" == onetest.klass.name %}
Expand Down
8 changes: 6 additions & 2 deletions fhirspec.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import glob
import json
import datetime
import pathlib

from logger import logger
import fhirclass
Expand Down Expand Up @@ -267,6 +268,9 @@ def write(self):

vsrenderer = fhirrenderer.FHIRValueSetRenderer(self, self.settings)
vsrenderer.render()

# Create init file so that our relative imports work out of the box
pathlib.Path(self.settings.tpl_resource_target, "__init__.py").touch()

if self.settings.write_factory:
renderer = fhirrenderer.FHIRFactoryRenderer(self, self.settings)
Expand Down Expand Up @@ -609,11 +613,11 @@ def needed_external_classes(self):
# look at all properties' classes and assign their modules
for prop in klass.properties:
prop_cls_name = prop.class_name
if prop.enum is not None:
if prop.enum is not None and not self.spec.class_name_is_native(prop_cls_name):
Comment on lines -612 to +616
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the bit that fixes AccountStatus.str being used instead of str

enum_cls, did_create = fhirclass.FHIRClass.for_element(prop.enum)
enum_cls.module = prop.enum.name
prop.module_name = enum_cls.module
if not enum_cls.name in needed:
if enum_cls.name not in needed:
needed.add(enum_cls.name)
needs.append(enum_cls)

Expand Down