-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
fix(server): People count after updating recognized faces count #13288
Conversation
Thank you for the PR. Can you please run |
Sure. I have run the command from root of project. Thank you |
@jedi04 This open another series of issues. |
Hey @jedi04, I appreciate this has been open for quite a while. Just trying to go through some of our older PRs and cleanup. Is this ready to merge? I did a quick look through and I didn't really understand why you changed the SQL to have a subquery. From a brief look I feel like you could've just changed the HAVING statement to handle the minimum faces setting. Is there any other advantage to changing the query in this way? |
Could you expand on what other issues this opens in your eyes? |
I saw that issue but it isn't really clear to me why this fix is directly a problem. It sounds like there are more issues in this area, but this PR feels like a bug regardless, it doesn't need to fix all of the problems. I think it's fine to look to merge this change but also look at a broader solution to the problems you mention separately. |
This only fix the returned counte of the people, by limiting the real count to those that only have a at least N faces. If I have a person Y with 3 faces the person is created. if I delete one of the pictures the person Y still exist. but the counter will return a lower count (as it dont meet the threshold). I could have already named the person. IMHO: limiting the count is not the correct way to fix this, the correct way is to ensure that the DB is consistent with the data. |
I now understand the issue. In that case I agree, and it's probably better to fix that underlying issue then. In which case I think I will close this issue as it is a bandaid to the wider issue. |
Hi, I have the same issue, but I'm totally lost in finding a solution... What should I do in this case? My minimal face recognition is set to 3... How can I solve the issue? |
This PR fixes #13245
It updates the query used to fetch the total count of people keeping in consideration the recognized faces setting.