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

Refactor ClassifAI so that it's easier to add more Providers under a Service. #404

Closed
1 task done
Sidsector9 opened this issue Mar 6, 2023 · 2 comments
Closed
1 task done
Labels
help wanted Extra attention is needed
Milestone

Comments

@Sidsector9
Copy link
Member

Sidsector9 commented Mar 6, 2023

Is your enhancement related to a problem? Please describe.

At the moment, the plugin offers services like Image Processing, Language Processing, etc. But the problem is when you add multiple Providers to a Service, accessing data for that Provider is not convenient.

See this line for example:

$provider = $service_manager->service_classes[ $service ]->provider_classes[0];

The direct index access of 0 makes it impossible to access settings for Providers other than the one at 0.

Helper functions such as:

function get_supported_post_statuses() {

...have utility outside of Watson's Language Understanding feature. In future, it we add let say, Microsoft's Text to Speech feature which is a Provider for the Language Processing Service, then the current helper get_supported_post_statuses() will not work even though it is required by the new Provider. This will force developers to add helpers for each Provider which will lead to code duplication.

Designs

No response

Describe alternatives you've considered

  1. Identify existing helper functions that can be useful for multiple Providers.
  2. Refactor (if possible) or deprecate in favour of functions that are generic and available to every Provider that needs it.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@Sidsector9 Sidsector9 changed the title EPIC: Refactor ClassifAI so that it's easier to add more Providers under a Service. Refactor ClassifAI so that it's easier to add more Providers under a Service. Mar 6, 2023
@jeffpaul jeffpaul added this to the 2.0.0 milestone Mar 6, 2023
@jeffpaul jeffpaul moved this from Incoming to To Do in Open Source Practice Mar 6, 2023
@jeffpaul jeffpaul added the help wanted Extra attention is needed label Mar 6, 2023
@dkotter
Copy link
Collaborator

dkotter commented Mar 8, 2023

I started some work on this in #405, see https://github.com/10up/classifai/pull/405/files#diff-502f71f01635ed43aac831e9a91f0097f7bd09bec4e36b7dc6b52af2b694a7a0R30. My plan is to clean this up even more once that PR is merged

@dkotter
Copy link
Collaborator

dkotter commented Mar 21, 2023

Note that PR is now merged and I've opened up a simple first step follow up in #413. That said, I think there will still be more needed, as I've seen other areas of the code that assume we only have a single provider.

@dkotter dkotter modified the milestones: 2.1.0, 2.2.0, Future Release May 1, 2023
@jeffpaul jeffpaul modified the milestones: Future Release, 3.0.0 Mar 5, 2024
@jeffpaul jeffpaul closed this as completed Mar 5, 2024
@github-project-automation github-project-automation bot moved this from To Do to Merged in Open Source Practice Mar 5, 2024
@jeffpaul jeffpaul moved this from Merged to Done/Released in Open Source Practice Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
Archived in project
Development

No branches or pull requests

3 participants