[AL-4398] Add Model Run exports #840

mnoszczak · 2023-01-23T10:36:13Z

Adds model run exports method method

msokoloff1 · 2023-01-30T23:18:31Z

labelbox/schema/model_run.py

+    """
+
+    def export_labels_v2(self, task_name: str,
+                         filter: Optional[ModelRunExportFilter]) -> Task:


You should pass in a default value of None for the filter. Even though the type hint says optional, it is a required arg here.

filter={"media_attributes": True}

hm, this does not look like a filter to me but rather a parameter of the export

I have renamed from ModelRunExportFilter to ModelRunExportParams

labelbox/schema/model_run.py

msokoloff1 · 2023-01-30T23:22:23Z

labelbox/schema/model_run.py

+                    "includeAttachments":
+                        filter["attachments"]


It is also weird that the gql fields are different than the name we expose in the SDK. I can see a future where this causes confusion :).

We should make these the same.

Would you update SDK or the API? Also, should SDK use camelCase or snake_case?

We should update the api. I think it is fine if the api casing is camelcase and sdk is snake case just that the names should be the same.

msokoloff1 · 2023-01-30T23:22:54Z

tests/integration/annotation_import/test_model_run.py

+        task_name, filter={"media_attributes: true"})
+    assert task.name == task_name
+    task.wait_till_done()
+    assert task.status == "COMPLETE"


It would be good to check at least that the high level keys exist in the payload and the number of items returned was expected.

Shouldn't that be in the scope of integration tests of API, not the SDK?

The SDK is the most useful end to end integration test. So it is nice to have it here to guarantee that things are working (also it blocks deployments). But technically it is weird that it serves that purpose.

labelbox/schema/model_run.py

attila-papai · 2023-01-31T08:54:59Z

labelbox/schema/model_run.py

+    """
+
+    def export_labels_v2(self, task_name: str,
+                         filter: Optional[ModelRunExportFilter]) -> Task:


filter={"media_attributes": True}

hm, this does not look like a filter to me but rather a parameter of the export

attila-papai · 2023-01-31T08:57:39Z

labelbox/schema/model_run.py

+
+    def export_labels_v2(self, task_name: str,
+                         filter: Optional[ModelRunExportFilter]) -> Task:
+        mutation_name = "exportDataRows"


I still think this general mutation should be removed and two specialized mutations be added instead:

exportDataRowsInProject

exportDataRowsInModelRun

Right now, the user can call this mutation without any filters and that would export all the data rows in his organization, and if he has 100M data rows, it's going to add a huge load to our system.

@attila-papai won't it be constrained to a specific model run? I think the consensus was to merge the SDK as it is (as API won't change, and then add API endpoints -> update SDK)

so when you do

model_run = client.get_model_run("model_run_id") model_run.export_labels_v2(...)

isn't that constrained to a single model run?

It is constrained to a single model run from within that SDK call:

"modelRunIds": [self.uid], "projectIds": [] },

so it would make sense if mutation exportDataRowsInModelRun accepts only a single model run id, because users can't export multiple model runs in a single call, right?
I can add this new mutation, shouldn't be more than half an hour.

@attila-papai https://github.com/Labelbox/intelligence/pull/13104 I've added it, but it accepts multiple modelRunIds, should we:
a) make it general purpose and rename to exportDataRowsInModelRuns
b) limit it to just one modelRunId and keep the exportDataRowsInModelRun name

msokoloff1 · 2023-01-31T18:49:40Z

labelbox/schema/model_run.py

+    
+    """
+
+    def export_labels_v2(self, task_name: str,


from @karenkyang:
We should call these functions .export_v2(...)

msokoloff1 · 2023-01-31T18:51:00Z

labelbox/schema/model_run.py

+    """
+    Creates a model run export task with the given filter and returns the task.
+    
+    >>>    export_task = export_labels_v2("my_export_task", filter={"media_attributes": True})


filter={"media_attributes": True})
^^ params

msokoloff1 · 2023-01-31T19:21:32Z

labelbox/schema/export_params.py

+class DataRowParams(TypedDict):
+    include_data_row_details: Optional[bool]
+    include_media_attributes: Optional[bool]
+    include_metadata_fields: Optional[bool]
+    include_attachments: Optional[bool]
+
+
+class ProjectExportParams(DataRowParams):
+    include_project_details: Optional[bool]
+    include_label_details: Optional[bool]
+    include_performance_details: Optional[bool]
+
+
+class ModelRunExportParams(DataRowParams):
+    # TODO: Add model run fields
+    pass


I feel like these should be pydantic models. It is really bothering me that we have yet another pattern for our interfaces. wdyt?

I think that we've discussed this point with Karen and ended up on picking TypedDict, because it requires less ceremony from the end user

mnoszczak requested a review from msokoloff1 January 23, 2023 10:36

Add Model Run exports

798f518

mnoszczak force-pushed the mno/al-4398 branch from 73c45f1 to 798f518 Compare January 23, 2023 10:40

mnoszczak added 2 commits January 23, 2023 11:53

Fix 3.7

18277f1

Fix 3.7 v2

2d89326

mnoszczak force-pushed the mno/al-4398 branch 2 times, most recently from d1c9aca to 45dc93a Compare January 23, 2023 11:18

Fix 3.7 v3

3fa9a9e

mnoszczak force-pushed the mno/al-4398 branch 2 times, most recently from 147583e to 8f0bf2c Compare January 23, 2023 11:28

Add test

b26f235

mnoszczak force-pushed the mno/al-4398 branch from 8f0bf2c to b26f235 Compare January 23, 2023 11:30

mnoszczak added 7 commits January 23, 2023 12:34

Fix 3.7 v4

d5b588a

Fix return type

304d8e9

Fix metadata fields

3d80556

Formatting

2f65e22

Fix task_id ref

4fd153e

Fix payload

8210978

Remove global filters

598db8d

mnoszczak force-pushed the mno/al-4398 branch from 2d965a9 to 598db8d Compare January 27, 2023 12:05

Fix test

2347a2b

mnoszczak force-pushed the mno/al-4398 branch from 4ffc435 to 2347a2b Compare January 30, 2023 08:53

mnoszczak requested a review from attila-papai January 30, 2023 09:00

Merge remote-tracking branch 'origin/develop' into mno/al-4398

3b065b8

msokoloff1 reviewed Jan 30, 2023

View reviewed changes

labelbox/schema/model_run.py Show resolved Hide resolved

msokoloff1 reviewed Jan 30, 2023

View reviewed changes

labelbox/schema/model_run.py Outdated Show resolved Hide resolved

attila-papai reviewed Jan 31, 2023

View reviewed changes

mnoszczak force-pushed the mno/al-4398 branch 2 times, most recently from b587d87 to 639501c Compare January 31, 2023 12:47

CR changes

dc7d985

mnoszczak force-pushed the mno/al-4398 branch from 639501c to dc7d985 Compare January 31, 2023 13:00

msokoloff1 reviewed Jan 31, 2023

View reviewed changes

mnoszczak force-pushed the mno/al-4398 branch 3 times, most recently from 50d2f4f to 4863f15 Compare February 2, 2023 01:38

Update tests

32c30f2

mnoszczak force-pushed the mno/al-4398 branch from 4863f15 to 32c30f2 Compare February 2, 2023 01:38

mnoszczak added 2 commits February 2, 2023 03:10

Rename params

a708c31

Revert back to TypedDict

c9d8265

mnoszczak requested review from attila-papai and msokoloff1 February 2, 2023 12:34

msokoloff1 approved these changes Feb 2, 2023

View reviewed changes

mnoszczak merged commit dbadecd into develop Feb 2, 2023

mnoszczak deleted the mno/al-4398 branch February 2, 2023 13:27

[AL-4398] Add Model Run exports #840

[AL-4398] Add Model Run exports #840

Uh oh!

Conversation

mnoszczak commented Jan 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

msokoloff1 Jan 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mnoszczak commented Jan 23, 2023 •

edited

Loading

msokoloff1 Jan 30, 2023 •

edited

Loading