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

Parameters for Expansions #1265

Closed
1 task done
jamlung-ri opened this issue Mar 28, 2022 · 30 comments
Closed
1 task done

Parameters for Expansions #1265

jamlung-ri opened this issue Mar 28, 2022 · 30 comments
Assignees
Labels
api2 OCL API v2 enhancement New feature or request web2 OCL WEB v2

Comments

@jamlung-ri
Copy link
Contributor

jamlung-ri commented Mar 28, 2022

Requirements/design work: parameter-driven expansions (both API and UI)

Missing pieces:

  • Ability to see what parameters were enabled for each expansion in the UI
@paynejd
Copy link
Member

paynejd commented Apr 8, 2022

Proposed parameters for Round 1:

  • filter
  • date
  • activeOnly
  • displayLanguage
  • exclude-system
  • system-version

Documentation here: https://docs.google.com/document/d/1z2h65pP934PyQg3_QSVp0pfHQaffwfTlkTDBXSIk5DI/edit#heading=h.h2hdgqorat4r

@snyaggarwal snyaggarwal self-assigned this Apr 13, 2022
snyaggarwal added a commit to OpenConceptLab/oclweb2 that referenced this issue Apr 13, 2022
snyaggarwal added a commit to OpenConceptLab/oclweb2 that referenced this issue Apr 13, 2022
snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue Apr 13, 2022
@snyaggarwal
Copy link
Contributor

snyaggarwal commented Apr 13, 2022

@paynejd @jamlung-ri Enabled "filter" parameter in TermBrowser -- right now it applies wildcard search after evaluating references. This is deployed on QA.
I have also added an option to view Expansion parameters (in a dialog in JSON format) on TermBrowser

@jamlung-ri
Copy link
Contributor Author

@snyaggarwal This seems to be working well!

One note: when viewing an expansion, the Filters chip seems to be in the wrong context. Example expansion where only a couple of concepts should be appearing but the facets and filters act as if there are many more concepts.

image

@paynejd
Copy link
Member

paynejd commented Apr 13, 2022 via email

snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue Apr 14, 2022
@snyaggarwal
Copy link
Contributor

@jamlung-ri Facets were not filtering on expansion, it's fixed.
@paynejd It's using full text search.

snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue Apr 14, 2022
snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue Apr 14, 2022
snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue Apr 14, 2022
snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue Apr 14, 2022
@jamlung-ri
Copy link
Contributor Author

@snyaggarwal Has this been deployed to QA? I just made a new collection and the issue is still there, but the fix might not have made it in yet.
image

snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue Apr 15, 2022
snyaggarwal added a commit to OpenConceptLab/oclweb2 that referenced this issue Apr 15, 2022
@jamlung-ri
Copy link
Contributor Author

jamlung-ri commented Apr 15, 2022

@snyaggarwal Has this been deployed to QA? I just made a new collection and the issue is still there, but the fix might not have made it in yet.

This is fixed now and working well! Will continue testing on more parameters once available.

snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue Apr 20, 2022
snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue Apr 20, 2022
…e system version considers valuesets as well
snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue Apr 21, 2022
…date datetime field and setting on version release
snyaggarwal added a commit to OpenConceptLab/oclweb2 that referenced this issue Apr 21, 2022
@snyaggarwal
Copy link
Contributor

snyaggarwal commented Apr 21, 2022

For date parameter:

  • revision_date will be a datetime field (earlier just date)
  • Source/Collection version forms/API need to accept revision_date
  • revision_date to auto set when a version is released
  • revision_date will be overridable for TermBrowser/API
  • data parameter in Expansion form need to accept any partial date (as per FHIR regex)

snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue Apr 21, 2022
snyaggarwal added a commit to OpenConceptLab/oclweb2 that referenced this issue Apr 21, 2022
snyaggarwal added a commit to OpenConceptLab/oclweb2 that referenced this issue Apr 21, 2022
@snyaggarwal
Copy link
Contributor

For date parameter:

  • revision_date will be a datetime field (earlier just date)
  • Source/Collection version forms/API need to accept revision_date
  • revision_date to auto set when a version is released
  • revision_date will be overridable for TermBrowser/API
  • data parameter in Expansion form need to accept any partial date (as per FHIR regex)

@jamlung-ri @paynejd this is available on QA

@jamlung-ri
Copy link
Contributor Author

Everything seems to be working with this. I was able to autogenerate a revision_date on a source version, edit it, and run an expansion with it. See here, where expansions e2 and e3 are the same expansion except the date parameter is used. https://app.qa.openconceptlab.org/#/users/ocladmin/collections/testjoe14Apr22/versions

Worth noting that the revision_date stays the same even if I unrelease the source version.

@jamlung-ri
Copy link
Contributor Author

I am good with closing this but @paynejd may want to do some testing

@snyaggarwal
Copy link
Contributor

@jamlung-ri there is still one parameter left "displayLanguage" I will try to read and add it today, if there are no blockers.

@snyaggarwal snyaggarwal added web2 OCL WEB v2 api2 OCL API v2 enhancement New feature or request labels Apr 27, 2022
@snyaggarwal
Copy link
Contributor

@jamlung-ri @paynejd All the parameters mentioned in this ticket are available on QA/Staging with the exception of "displayLanguage". For that we need to discuss and create a separate ticket with other round 2 parameters.

@jamlung-ri
Copy link
Contributor Author

jamlung-ri commented Apr 27, 2022

Testing log with this collection:

  • Using "exclude-system": "https://datim.org/CodeSystem/MER" - created an expansion that contained no MER concepts
  • Using "system-version": "http://worldhealthorganization.github.io/ddcc/ConceptMap/WHO_DDCC_Map_ICD11_to_SCT_Disease_Targeted|v1" - created an expansion that only contains a particular version of a source (i.e. 1 mapping from v1 instead of 3 mappings)
    • Unexpected Behavior: In my mind, this should only constrain the source that I specified, but it constrained the entire collection to only use that source and version. Meaning, in my mind there should be other concepts from other sources too, but there are not. It seems like this parameter is filtering sources, not just the specified source version. Am I misunderstanding the intent of this parameter, or is this a bug?
    • (Addressed) Unexpected Behavior: When using this parameter to constrain to only one older version (v4), it should be giving me 3 concepts in this expansion. However, I am only getting 2 concepts.
  • Using "filter": "blood" - created an expansion that only contains concepts with "blood" in it
  • Using filter "date": "2022-04-XX" - created multiple expansions (April-2, April-3, April 4, April-5, April-6) based on revision dates of this source - most appeared as expected

cc: @snyaggarwal

@snyaggarwal
Copy link
Contributor

snyaggarwal commented Apr 28, 2022

Unexpected Behavior: When using this parameter to constrain to only one older version (v4), it should be giving me 3 concepts in this expansion. However, I am only getting 2 concepts.

Using filter "date": "2022-04-XX" - created multiple expansions (April-2, April-3, April 4, April-5, April-6) based on revision dates of this source - most appeared as expected
Unexpected Behavior: The collection versions April-3 and April-4 should each be showing 3 concepts (see underlying source versions), but instead they are only showing 2.

_

@jamlung-ri this is working fine, the reason you see 2 concepts and not 3 is because of the following:

  1. The reference added in the collection is /users/ocladmin/sources/test-joe/concepts/?page=1&limit=25, which returns 3 concepts -- numeric, 435 and new-fake
  2. new-fake is not there in v4, there was a fake in v4 which was later retired.
  3. the intersection of the above two returns numeric and 435 which is what expansion is showing
  4. new-fake is not in the expansion because its not there in v4, and fake is not there in the expansion because its not a result of reference now (after getting retired)

@snyaggarwal
Copy link
Contributor

_

Unexpected Behavior: In my mind, this should only constrain the source that I specified, but it constrained the entire collection to only use that source and version. Meaning, in my mind there should be other concepts from other sources too, but there are not. It seems like this parameter is filtering sources, not just the specified source version. Am I misunderstanding the intent of this parameter, or is this a bug?

_

@jamlung-ri Yes thats what its doing, its including only that version, same goes for date or filter parameters.
My understanding was that these parameters needs to be applied on the results after reference evaluation and only include that matches the parameter criteria.
@paynejd Can you please confirm if this is the right behaviour?

@jamlung-ri
Copy link
Contributor Author

Unexpected Behavior: When using this parameter to constrain to only one older version (v4), it should be giving me 3 concepts in this expansion. However, I am only getting 2 concepts.

Using filter "date": "2022-04-XX" - created multiple expansions (April-2, April-3, April 4, April-5, April-6) based on revision dates of this source - most appeared as expected
Unexpected Behavior: The collection versions April-3 and April-4 should each be showing 3 concepts (see underlying source versions), but instead they are only showing 2.

_

@jamlung-ri this is working fine, the reason you see 2 concepts and not 3 is because of the following:

1. The reference added in the collection is `/users/ocladmin/sources/test-joe/concepts/?page=1&limit=25`, which returns 3 concepts -- `numeric`, `435` and `new-fake`

2. `new-fake` is not there in v4, there was a `fake` in v4 which was later retired.

3. the intersection of the above two returns `numeric` and `435` which is what expansion is showing

4. `new-fake` is not in the expansion because its not there in v4, and `fake` is not there in the expansion because its not a result of reference now (after getting retired)

Awesome, thanks for confirming. That makes sense now. Consider those pieces addressed!

@paynejd
Copy link
Member

paynejd commented Apr 29, 2022

@jamlung-ri @snyaggarwal Can you outline the rules for when/how revision_date is set automatically?

@snyaggarwal
Copy link
Contributor

  1. revision_date can be specified while creating a repo version
  2. revision_date is auto set to now when a version is released (if it's not already set)
  3. It can be changed via editing a repo version from TermBrowser or using PUT API for repo version.

@paynejd
Copy link
Member

paynejd commented Apr 29, 2022

Unexpected Behavior: In my mind, this should only constrain the source that I specified, but it constrained the entire collection to only use that source and version. Meaning, in my mind there should be other concepts from other sources too, but there are not. It seems like this parameter is filtering sources, not just the specified source version. Am I misunderstanding the intent of this parameter, or is this a bug?
@jamlung-ri Yes thats what its doing, its including only that version, same goes for date or filter parameters.
My understanding was that these parameters needs to be applied on the results after reference evaluation and only include that matches the parameter criteria.
@paynejd Can you please confirm if this is the right behaviour?

system-version is intended to influence how the OCL API evaluates references, rather than being a filter applied after the references have been evaluated. Consider these references:

/orgs/CIEL/sources/CIEL/concepts/1234/
/orgs/CIEL/sources/CIEL/v2021-03-12/concepts/5678/
/users/ocladmin/sources/custom-concepts/concepts/123/

If I generate an expansion with no parameters, these references will be resolved to the following repo versions:

/orgs/CIEL/sources/CIEL/concepts/1234/ ==> /orgs/CIEL/sources/CIEL/latest/ ==> /orgs/CIEL/sources/CIEL/v2022-03-19/
/orgs/CIEL/sources/CIEL/v2021-03-12/concepts/5678/ ==> /orgs/CIEL/sources/CIEL/v2021-03-12/  # no change
/users/ocladmin/sources/custom-concepts/concepts/123/ ==> /users/ocladmin/sources/custom-concepts/latest/ ==> /users/ocladmin/sources/custom-concepts/v1/

If I generate an expansion with system-version=https://ciel.org/CodeSystem/|v2021-12-15, references will now resolve to these repo versions -- note that all references are still applicable, however, because system-version is only changing how references to CIEL are evaluated:

# This one is forced to use the version in the `system-version` parameter
/orgs/CIEL/sources/CIEL/concepts/1234/ ==> /orgs/CIEL/sources/CIEL/v2021-12-15/

# This one is unchanged since the version is explicit in the reference
/orgs/CIEL/sources/CIEL/v2021-03-12/concepts/5678/ ==> /orgs/CIEL/sources/CIEL/v2021-03-12/  # no change

# This is unchanged because the `system-version` parameter does not apply to it
/users/ocladmin/sources/custom-concepts/concepts/123/ ==> /users/ocladmin/sources/custom-concepts/latest/ ==> /users/ocladmin/sources/custom-concepts/v1/

snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue May 30, 2022
snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue May 30, 2022
@snyaggarwal
Copy link
Contributor

@jamlung-ri @paynejd This is deployed on QA

@jamlung-ri
Copy link
Contributor Author

jamlung-ri commented May 31, 2022

Testing log for 31 May 2022:

  • Goal: Simple expansion using this collection:

    • Attempt 1: Create an expansion that only includes v1 - Outcome: Success
    • Attempt 2: Create an expansion that only includes v2 - Outcome: Mostly success
      • Note: Version v2 has a different canonical url than later versions. I could not use v2's canonical url and instead had to use the updated canonical url from the later versions.
    • Attempt 3: Create an expansion that only includes v5 - Outcome: Success
  • Goal: More complex expansion using this collection (which is made up of Source A and Source B):

    • Part 1: Create an expansion that only includes v5 of Source A - Outcome: Fails.
      • Issue: This expansion should only include concepts from v5, but it seems to be getting concepts from v4 (note that the concept "fake" is not retired, which it should be in v5). Is this perhaps because v4 is the latest released version?
    • Part 2: Create an expansion that only includes v4 of source B - Outcome: Success
    • Part 3 - Create an expansion that includes only v5 of Source A and v4 of source B - Outcome: Fails
      • Issue: Likely a syntax issue with the "system-version" parameter. @snyaggarwal How would I do more than one system version constraint on a single expansion?

@snyaggarwal
Copy link
Contributor

@jamlung-ri Right now you can only specify one system-version in parameters. Does FHIR provide a syntax to add multiple?

@jamlung-ri
Copy link
Contributor Author

Not seeing an example syntax specifically, but I do see on the FHIR spec that multiple system-version parameters can be included (i.e. it has 0..* cardinality). Wish there were some examples somewhere :/

@snyaggarwal
Copy link
Contributor

May be we can use , to separate multiple values.

@paynejd
Copy link
Member

paynejd commented Jun 16, 2022 via email

@snyaggarwal
Copy link
Contributor

@paynejd in terms of request, repeat means systemVersion=[..]?

@paynejd
Copy link
Member

paynejd commented Oct 5, 2022

@snyaggarwal Following up on specifying more than one system-version-- we either delimit with commas, or we repeat the URL parameter, e.g.

?system-version=https://ciel.org/CodeSystem/|v2021-12-15,https://who.int/icd10|v2.9.32

?system-version=https://ciel.org/CodeSystem/|v2021-12-15&system-version=https://who.int/icd10|v2.9.32

@rkorytkowski Have you found any examples of how to provide multiple request parameters from your work on the OCL FHIR Core (i.e. for a parameter that has 0..* cardinality)? We'd like this to align with FHIR spec on this.

@snyaggarwal
Copy link
Contributor

?system_version[]=https://foo.com&system_version[]=https://bar.com
or
?system_version=https://foo.com,https://bar.com

@jamlung-ri
Copy link
Contributor Author

Following up on this work in #1406 - closing out this one since expansion parameters are implemented in OCL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api2 OCL API v2 enhancement New feature or request web2 OCL WEB v2
Projects
None yet
Development

No branches or pull requests

3 participants