miracum / ahd2fhir

A REST service for mapping text analysis results from Averbis Health Discovery to FHIR resources.
Apache License 2.0
8 stars 0 forks source link

Add support for AHD v6 #19

Closed chgl closed 1 year ago

uklft commented 3 years ago

Here is an overview of the changes:
https://help.averbis.com/pages/viewpage.action?pageId=40799961

uklft commented 3 years ago

I am currently playing around with Version 6.0 and 6.1. Some notes:

uklft commented 3 years ago

The change of the Key for ConceptIDs could temporarily be handled with

concept = annotation.get("conceptID", annotation.get("conceptId"))

In the future we could set the AHD version in the config and ask for the corresponding key.
As i am currently working on a way to pass the config to the mappers, this could be done easily (see mapper_handler.py)

chgl commented 3 years ago

I think we should decide whether to provide backwards compatibility or go all in on v6 and avoid having to support multiple versions. If we decide on the former, we might consider https://refactoring.guru/design-patterns/strategy/python/example to avoid cluttering each mapping implementation with conditionals based on the version (if it turns out to be excessive, that is).

In the case of conceptId we could also lowercase all keys in the dict.

chgl commented 2 years ago

are we sure this is completely resolved? https://github.com/miracum/ahd2fhir/pull/60 (I left some minor comments) doesn't seem to address the casing of conceptId or the lack of a version in the ahd payload.

The current unit tests, while at least hitting most of the code, aren't exactly meaningful in their assertions since they mostly just do assert x is not None. Snapshot testing may be a more comprehensive way to check that a given ahd payload (of either version) matches a possibly quite large FHIR output in its entirety. I don't have experience with it in Python, but https://pypi.org/project/pytest-snapshot/ looks promising, particularly https://github.com/joseph-roitman/pytest-snapshot#common-use-case.

miracum-bot commented 1 year ago

:tada: This issue has been resolved in version 3.0.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: