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

Fixed an issue to make sure all the top level categories are selected from aws rekognition response #67

Merged
merged 4 commits into from
Sep 17, 2021

Conversation

SiddharthV1
Copy link
Contributor

@SiddharthV1 SiddharthV1 commented Sep 16, 2021

added a function to the implementation of Label for correctly getting the top-level category returned by aws, this improves on the existing solution of only looking for the top-level category in Label.ParentName, by looking at both ParentName and Name

…aws, this improves on the existing solution of only looking for the top-level category in Label.ParentName, this is achieved by lookin at both ParentName and Name
@codecov-commenter
Copy link

Codecov Report

Merging #67 (234f0e6) into main (d756998) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #67   +/-   ##
=======================================
  Coverage   55.35%   55.35%           
=======================================
  Files           4        4           
  Lines         336      336           
  Branches       94       94           
=======================================
  Hits          186      186           
  Misses         70       70           
  Partials       80       80           

Comment on lines 17 to 20
match self.ParentName.is_empty() {
true => &self.Name,
_ => &self.ParentName,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change this to an if else

@@ -24,7 +33,7 @@ impl RekognitionResponse {
let labels: HashSet<String> = self
.ModerationLabels
.iter()
.map(|l| l.ParentName.clone())
.map(|l| l.top_category().to_owned())
.filter(|l| !l.is_empty())
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this line necessary since a empty check is occurring in the top_category fn?

@bhaskarkishore bhaskarkishore merged commit 7138cd1 into main Sep 17, 2021
@bhaskarkishore bhaskarkishore deleted the fix/categories branch January 27, 2022 11:41
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