feat(medcat-service): Medcat Demo and AnonCAT demo uplift in Gradio#279
feat(medcat-service): Medcat Demo and AnonCAT demo uplift in Gradio#279alhendrickson merged 26 commits intomainfrom
Conversation
…nd improve test coverage in test_demo_logic.py
…entity resolution
mart-r
left a comment
There was a problem hiding this comment.
Overall I think this looks great!
Left a few comments on type hints for return types.
And one on the long-winded setup for the demo web interface that I think could use some refactoring for better readability and to reduce nesting.
| ] | ||
|
|
||
|
|
||
| def perform_named_entity_resolution(input_text: str, redact: bool | None = None): |
There was a problem hiding this comment.
Perhaps adding the return type a type hint here would be useful? You've descried it in the docs anyway (though it's missing the last str).
I.e -> tuple[dict, list[list[str]], str]
| return response_tuple | ||
|
|
||
|
|
||
| def medcat_demo_perform_named_entity_resolution(input_text: str): |
There was a problem hiding this comment.
Again, including the return type (-> tuple[dict, list[list[str]]]) could be useful.
| return result[0], result[1] | ||
|
|
||
|
|
||
| def anoncat_demo_perform_deidentification(input_text: str, redact: bool): |
There was a problem hiding this comment.
Again, return type might be useful.
| return annotation_details_placeholder_text | ||
|
|
||
|
|
||
| if settings.deid_mode: |
There was a problem hiding this comment.
This feels a little off to me. I suppose global state is to be expected here, but I wouldn't expect as much code to exist just within the module scope.
Both these (if / else) follow a pretty similar pattern (gr.Block -> gr.Row -> gr.Column), perhaps there's a way to simplify these to avoid some of the duplication?
But perhaps I'd just feel better if the logic (i.e the DeID mode and the regular NER+L mode) were just in their own separate methods that were called here.
The other thing to note is that there's quite a bit of nesting here (6 layers deep). Perhaps we can mitigate that a little? Perhaps we could split out some parts to separate methods?
Screenshots
Before
After
After: Anoncat demo
Split into two tabs to show details. First tab should be at least as good as existing one