Skip to content

Conversation

@tomolopolis
Copy link
Member

  • model pack selection, now most if not all deployments will not be using CDB / Vocab pairs
  • new ClinicalText component, so entering example text and viewing the annotated version is on the same looking component
  • improved filtering via use of the concept picker, assumes the model used is the same cdb_search_filter used for
  • Meta annotations for each selected concept is also now displayed

Copy link
Collaborator

@mart-r mart-r left a comment

Choose a reason for hiding this comment

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

A few minor questions / comments.
Didn't look at the vue or ts stuff - if it displyas it displays.

Looks good overall.

try:
cat = get_medcat_from_model_pack_id(int(modelpack_id))
except (ValueError, TypeError):
return HttpResponseBadRequest('Invalid modelpack_id')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add the ID in the exception?

except (ValueError, TypeError):
return HttpResponseBadRequest('Invalid modelpack_id')
except ModelPack.DoesNotExist:
return HttpResponseBadRequest('ModelPack does not exist')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add the ID in the exception?

logger.warning(f'Failed to get children for CUI {parent_cui}: {e}')
cuis_set = expanded_cuis

cat.config.components.linking.filters.cuis = cuis_set
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess this was always the case, but could this potentially cause issues? I.e since the same model packs instances are (potentially) used across multiple projects at once, I could see this creating unexpected filtering.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah good point, fixed

logger.warning(f'Failed to get children for CUI {parent_cui}: {e}')
cuis_set = expanded_cuis

curr_cuis = cat.config.components.linking.filters
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps a context manager would be better here?
I.e using https://docs.cogstack.org/projects/nlp/en/latest/autoapi/medcat/utils/config_utils/index.html#medcat.utils.config_utils.temp_changed_config
something like:

with temp_changed_config(cat.config.components.linking, 'filters', cuis_set):
    spacy_doc = cat(message)

@tomolopolis tomolopolis merged commit cbd883e into main Jan 20, 2026
10 checks passed
@tomolopolis tomolopolis deleted the improve-demo-page branch January 20, 2026 15:36
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.

3 participants