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: Updates type for completed_by field #420

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Burhan-Q
Copy link

Problem

Annotation type field is currently only valid for int types, however API returns dict for completed_by field. Dictionary returned by API can't be used directly with Annotation object.

label-studio-sdk==1.0.10
pydantic==2.10.6

Changes

  • Added import for UserSimple model.
  • Updated type annotation to use typing.Union with int and UserSimple

Impact

  • Now direct output from API can be accepted by Annotation type.
  • Maintains compatibility with accepting int type as well.

Checked with

Simple test to validate that dict and int values are accepted by Annotation type for completed_by field.

from label_studio_sdk.client import LabelStudio
from label_studio_sdk.types import Annotation

client = LabelStudio()

annotations = client.annotations.get(id=12345)
first = next(annotations)

isinstance(first.get("completed_by"), dict)
>>> True

as_int = {
    k: v for k,v in first.items() if k != "completed_by"
} | {"completed_by": 09876}

annotation_01 = Annotation(**first)
annotation_02 = Annotation(**as_int)

isinstance(annotation_01.completed_by, UserSimple)
>>> True

isinstance(annotation_02.completed_by, UserSimple)
>>> True

Note

It appears that these might be generated from API documentation, which might mean this change might not integrate permanently if merged. Unclear to me at the moment if there is a better way to integrate a change like this into the current workflow, but it seemed like it was worthwhile to open a PR as a proposal for a QOL improvement.

@Burhan-Q Burhan-Q changed the title Updates type for completed_by field fix: Updates type for completed_by field Feb 20, 2025
@smohan123
Copy link

smohan123 commented Feb 27, 2025

Hi Burhan, Samir here. Thanks for your PR. Engineering has evaluated your proposal and agrees with the idea. However, as you might have suspected, this is an auto-generated class definition, so we need to make the change internally elsewhere in order to effect the change that you proposed. I have created a Jira ticket referencing your PR and very good change notes. Max will likely comment here once the work has been completed as well as actioning on the PR itself. Of course, you're likely to see the update only in a subsequent version of the SDK, so please use your local modified copy for now. Thanks for your submission, it's a great idea and was agreed upon immediately on our side.

@Burhan-Q
Copy link
Author

@smohan123 thanks! I did open a PR for the generation of the API/SDK HumanSignal/label-studio-client-generator#64 but it only reflects my best guess of what's needed to make it work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants