Skip to content

More face attributes for microsoft_face_detect API#51028

Closed
gonzalezcalleja wants to merge 8 commits intohome-assistant:devfrom
gonzalezcalleja:add_more_attributes_microsoft_face_detect
Closed

More face attributes for microsoft_face_detect API#51028
gonzalezcalleja wants to merge 8 commits intohome-assistant:devfrom
gonzalezcalleja:add_more_attributes_microsoft_face_detect

Conversation

@gonzalezcalleja
Copy link
Copy Markdown
Contributor

@gonzalezcalleja gonzalezcalleja commented May 24, 2021

Breaking change

Proposed change

The actual component microsoft_face_detect are only using a few face attributes from Microsoft Face API, could be useful to add some other to the component.
In this PR I'm adding this face attributes:

  • hair: hair: group of hair values indicating whether the hair is visible, bald, and hair color if hair is visible
  • smile: smile intensity, a number between [0,1].
  • facialHair: return lengths in three facial hair areas: moustache, beard and sideburns. The length is a number between [0,1]. 0 for no facial hair in this area, 1 for long or very thick facial hairs in this area.
  • emotion: emotion intensity, including neutral, anger, contempt, disgust, fear, happiness, sadness and surprise.
  • accessories: accessories around face, including 'headwear', 'glasses' and 'mask'. Empty array means no accessories detected. Note this is after a face is detected. Large mask could result in no face to be detected.

https://westus.dev.cognitive.microsoft.com/docs/services/563879b61984550e40cbbe8d/operations/563879b61984550f30395236

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml
- platform: microsoft_face_detect
  scan_interval: 10000
  source:
    - entity_id: camera.calle
  attributes:
    - age
    - gender
    - glasses
    - hair
    - smile
    - facialhair
    - emotion
    - accessories

Additional information

Checklist

  • The code change is tested and works locally.
  • [] Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@KapJI
Copy link
Copy Markdown
Member

KapJI commented May 24, 2021

Shouldn't these constants be used somehow?

@gonzalezcalleja
Copy link
Copy Markdown
Contributor Author

Shouldn't these constants be used somehow?

If you did't declare it in image_processing.microsoft_face_detect, you can't use it in the config:

2021-06-20 19:06:26 ERROR (MainThread) [homeassistant.config] Invalid config for [image_processing.microsoft_face_detect]: Invalid attribute hair for dictionary value @ data['attributes']. Got ['age', 'gender', 'glasses', 'hair', 'smile', 'facialhair', 'emotion', 'accessories']. (See /etc/home-assistance/config/imageprocesing.yaml, line 207). Please check the docs at https://www.home-assistant.io/integrations/microsoft_face_detect 2021-06-20 19:06:26 ERROR (MainThread) [homeassistant.config] Invalid config for [image_processing.microsoft_face_detect]: Invalid attribute hair for dictionary value @ data['attributes']. Got ['age', 'gender', 'glasses', 'hair', 'smile', 'facialhair', 'emotion', 'accessories']. (See /etc/home-assistance/config/imageprocesing.yaml, line 221). Please check the docs at https://www.home-assistant.io/integrations/microsoft_face_detect

@KapJI
Copy link
Copy Markdown
Member

KapJI commented Jul 3, 2021

Please format your code with black and isort.

Copy link
Copy Markdown
Member

@janiversen janiversen left a comment

Choose a reason for hiding this comment

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

LGTM.

@frenck frenck added the smash Indicator this PR is close to finish for merging or closing label Nov 14, 2021
@allenporter
Copy link
Copy Markdown
Contributor

These are string specific to this API? It is not clear that sharing these string constants with a common library is worthwhile since the strings are specific to the microsoft API.

ATTR_FACES = "faces"
ATTR_GENDER = "gender"
ATTR_GLASSES = "glasses"
ATTR_HAIR = "hair"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree, these should not be added to default integration. Add to microsoft integration instead.

@frenck frenck removed the smash Indicator this PR is close to finish for merging or closing label Dec 3, 2021
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 3, 2022

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@github-actions github-actions bot added the stale label Mar 3, 2022
@github-actions github-actions bot closed this Apr 28, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants