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

fix: DIA-1637: Check object tag compatibility for NER tags #275

Merged
merged 3 commits into from
Dec 10, 2024

Conversation

hakan458
Copy link
Contributor

@hakan458 hakan458 commented Dec 9, 2024

We are filtering in LabelStudioSkill.validate_response_model based on allowed_control_tags and allowed_object_tags but not overwriting self.label_config based on this - only self.field_schema

Now checking to make sure any Labels tag points to a compatible object tag

@codecov-commenter
Copy link

codecov-commenter commented Dec 9, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 67.98%. Comparing base (a0f42f5) to head (c40fb95).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
adala/skills/collection/label_studio.py 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #275      +/-   ##
==========================================
- Coverage   68.02%   67.98%   -0.05%     
==========================================
  Files          47       47              
  Lines        2508     2511       +3     
==========================================
+ Hits         1706     1707       +1     
- Misses        802      804       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -53,7 +53,11 @@ def ner_tags(self) -> List[ControlTag]:
for tag_name in control_tag_names:
tag = self.label_interface.get_control(tag_name)
if tag.tag.lower() in {"labels", "hypertextlabels"}:
tags.append(tag)
if self.allowed_object_tags:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm trying to remember why we don't just always pass allowed_object_tags... do we have a good reason that this shouldn't always just be set, or have we kinda just stumbled into having to support both of these cases?
(I know we're not gonna fix it as part of this PR if that's the case, just want to make sure I understand here)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/HumanSignal/label-studio-enterprise/blob/develop/label_studio_enterprise/lse_ml_models/skills.py#L475

This is the reason I guess. I dont know the actual details of how passing it would drop information, since we only save filtered field_schema and keep the entire label_config anyway

@matt-bernstein matt-bernstein merged commit bf4aa1e into master Dec 10, 2024
11 checks passed
@matt-bernstein matt-bernstein deleted the fb-dia-1637 branch December 10, 2024 14:25
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.

5 participants