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

Ensure custom Classification Providers can fully work #762

Merged
merged 1 commit into from
May 3, 2024

Conversation

dkotter
Copy link
Collaborator

@dkotter dkotter commented Apr 19, 2024

Description of the Change

We recently went through a refactor of ClassifAI to make it easier to add additional Providers and Features. As part of this process, we more fully merged the two Classification approaches we have: classifying by suggesting new terms and classifying based on existing terms.

I recently was trying to add a new 3rd party Classification Provider and ran into a few issues that prevented this from fully working. We have a few places within ClassifAI that are hardcoded based on the two built-in Classification Providers. This means if you add a custom Provider, this code doesn't ever run. The two areas were:

  1. The admin preview functionality. This loads some JS code based on if the Provider is one of the two built-in Providers
  2. Actually saving results. The code here runs custom save methods based on the Provider, but only accounts for the two built-in Providers

This PR attempts to fix both of these problems. A new filter is introduced (classifai_feature_classification_pre_save_results), that allows you to modify the classification results before they're saved. This can be used by a custom Provider to process and save these results however is needed (can also be used to modify the results before a built-in Provider saves them, if desired).

For the previewer, I debated on a few approaches but ended up going pretty simple. If the Provider is IBM Watson, we load some custom code. If the Provider isn't IBM Watson, we load our other code. Previously we were loading this code if the Provider was OpenAI Embeddings. This means for a custom Provider, this second code block will be loaded. As long as the custom Provider follows the approach of our OpenAI Embeddings Provider, this should work fine.

How to test the Change

Not a whole lot to test here but can test that the preview functionality still works for the two built-in Providers.

Changelog Entry

Added - New filter, classifai_feature_classification_pre_save_results, that allows you to filter the classification results before they're saved, either modifying those results or running custom save routines.
Fixed - Ensure the classification admin preview functionality loads for non built-in Providers

Credits

Props @dkotter

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

… (or change the results that a built-in Provider returns). Also allow the preview functionality to work for custom Providers
@dkotter dkotter added this to the 3.1.0 milestone Apr 19, 2024
@dkotter dkotter self-assigned this Apr 19, 2024
@dkotter dkotter requested review from jeffpaul and a team as code owners April 19, 2024 17:13
@github-actions github-actions bot added the needs:code-review This requires code review. label Apr 19, 2024
Copy link
Member

@iamdharmesh iamdharmesh left a comment

Choose a reason for hiding this comment

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

PR LGTM and it tests well. Thanks for working on this @dkotter.

@iamdharmesh iamdharmesh merged commit 02c60a3 into develop May 3, 2024
16 checks passed
@iamdharmesh iamdharmesh deleted the fix/allow-new-classification-providers branch May 3, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs:code-review This requires code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants