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

CU-2exy49p: Make sure the cdb.add_concept really adds a concept or somehow make it clear #370

Merged
merged 2 commits into from
Nov 27, 2023

Conversation

adam-sutton-1992
Copy link
Collaborator

As per clickup.

Make sure the cdb.add_concept really adds a concept or somehow make it clear

Function is a bit confusing as it in fact requires the names to be pre-processed before it can be used.

Changed cdb.add_concept to a protected method, and changed all calls to it within MedCAT as appropriate.

I've also added a line of doucmentation pointing to to the helper function medcat.preprocessing.cleaners.prepare_name.

add_concept cannot be made private currently due to it useage. Conversely prepare_name cannot be used in the method as the cdbmaker.config is not accessible.

It might be worth checking with users that they're not using add_concept before this is merged.

@adam-sutton-1992 adam-sutton-1992 self-assigned this Nov 3, 2023
@adam-sutton-1992 adam-sutton-1992 changed the title Make sure the cdb.add_concept really adds a concept or somehow make it clear CU-2exy49p: Make sure the cdb.add_concept really adds a concept or somehow make it clear Nov 3, 2023
@tomolopolis
Copy link
Member

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.

Looks good. But we really should be leaving a deprecated method for backwards compatibility.

medcat/cdb.py Show resolved Hide resolved
medcat/cdb.py Show resolved Hide resolved
@adam-sutton-1992
Copy link
Collaborator Author

I've resolved the one change regarding documentation as it was done on a later line.

You can check the addition of the add_concept method and the deprecated tag.

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.

Looks good to me.

@adam-sutton-1992 adam-sutton-1992 merged commit b0ecd83 into master Nov 27, 2023
5 checks passed
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