Problem/Motivation

Currently maxResults values for Label Detection is hardcoded, making impossible for a user to customize this value based on a per-project basis. It would be nice if this could be exposed to a configuration value.

Proposed resolution

Expose this maxResults value for Label Detection used in the API calls to be configured by the site-builder.

Remaining tasks

None

User interface changes

None

API changes

None

Data model changes

None

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

marcoscano created an issue. See original summary.

marcoscano’s picture

Assigned: marcoscano » Unassigned
Status: Active » Needs review
FileSize
10.05 KB

Status: Needs review » Needs work

The last submitted patch, 2: expose-max-results-to-config-2783121-2.patch, failed testing.

naveenvalecha’s picture

Thanks for the patch!

  1. +++ b/src/Form/GoogleVisionSettingsForm.php
    @@ -50,10 +50,79 @@ class GoogleVisionSettingsForm extends ConfigFormBase {
    +    $form['max_results_fieldset'] = [
    

    change form key to 'max_results'

  2. +++ b/src/Form/GoogleVisionSettingsForm.php
    @@ -50,10 +50,79 @@ class GoogleVisionSettingsForm extends ConfigFormBase {
    +    $form['max_results_fieldset']['max_results_label_detection'] = [
    

    don't append the max_results before with every form key.

    Make it simple 'label_detection'
    Address at other places as well.

marcoscano’s picture

Assigned: Unassigned » marcoscano
marcoscano’s picture

Assigned: marcoscano » Unassigned
Status: Needs work » Needs review
FileSize
4.85 KB
11.35 KB

Addressed feedback in #4
Thanks!

ajalan065’s picture

Assigned: Unassigned » ajalan065

Status: Needs review » Needs work

The last submitted patch, 6: expose-max-results-to-config-2783121-6.patch, failed testing.

marcoscano’s picture

Status: Needs work » Needs review
FileSize
521 bytes
11.35 KB

Sorry there was a typo in the schema definition, now fixed

ajalan065’s picture

Assigned: ajalan065 » naveenvalecha
FileSize
7.3 KB
15.08 KB

Hi,
I would like to focus light on some points:
1. Only the Label Detection feature of the Google Vision API should be configurable. These are related to taxonomy tagging, hence, I agree to the point that it should be in the hands of the user.

However, other features need not be configurable, as it would be of no use. Here is why?
1. Landmark, Logo, Optical Character Detection have been used to fill the Alt Text attribute of the image file. So, IMHO, its of no use to leave it on the end user.
2. Safe Search detection feature is used to detect and avoid any explicit contents in images being uploaded. Hence, no use of making it configurable to have many values. It does not actually need any manipulation on the hands of the user.
3. Same is the case of User Emotion Detection.
4. As far as Image Properties is concerned, the image would have only one dominant color, either Red, Green or Blue or in the worst case, all the three (if the image is black and white).

We have hard coded the 'maxResults' with those values, as they would suffice our demand and requirements to use the features.

Here, I am uploading the new patch with necessary changes to reflect the configuration. Also, I have added and updated the tests to verify whether the value is configurable or not.

ajalan065’s picture

Title: Make max results per feature configurable instead of hardcoded » Make max results for Label Detection configurable instead of hardcoded
Issue summary: View changes
naveenvalecha’s picture

Title: Make max results for Label Detection configurable instead of hardcoded » Make max results for Label Detection configurable
Assigned: naveenvalecha » Unassigned
Category: Feature request » Task

leaving this open for the thoughts of eugene on this, what he thinks about #10

eugene.ilyin’s picture

I prefer the solution of Arpit. His explanation in #10 sounds pretty logical.

naveenvalecha’s picture

Status: Needs review » Fixed
eugene.ilyin’s picture

Committed.

  • eugene.ilyin committed f7b78d8 on 8.x-1.x
    Issue #2783121 by marcoscano, ajalan065, eugene.ilyin: Make max results...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.