Skip to content

[AL-5662] Add batch export v2 #1116

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jun 6, 2023
Merged

[AL-5662] Add batch export v2 #1116

merged 4 commits into from
Jun 6, 2023

Conversation

mnoszczak
Copy link
Contributor

@mnoszczak mnoszczak commented May 25, 2023 •
edited
Loading 

...
batches = project.batches()
batch = #find_interesting_batch
task = project.export_v2(filters={ "batch_id": batch.id  })
...

@mnoszczak mnoszczak requested a review from kopreschko May 25, 2023 15:08
@mnoszczak mnoszczak force-pushed the mno/al-5662 branch 4 times, most recently from 05841f5 to df814eb Compare May 25, 2023 16:24
kkim-labelbox
kkim-labelbox reviewed May 25, 2023
View reviewed changes
tests/integration/test_batch.py Outdated
@@ -1,6 +1,10 @@
from labelbox.exceptions import ProcessingWaitTimeout
import pytest
from labelbox import Dataset, Project
from typing import Tuple
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecesary imports?

kkim-labelbox
kkim-labelbox approved these changes Jun 1, 2023
View reviewed changes
Copy link
Contributor

@kkim-labelbox kkim-labelbox left a comment

Choose a reason for hiding this comment

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

LGTM. Left a minor comment, i'll leave it up for you to fix.
Looks like we're only adding it to a project for now, but we may also want to support it to catalog exports in the future, but we'll leave that for another day

tests/integration/test_project.py
@@ -42,6 +45,43 @@ def test_project(client, rand_gen):
assert project not in projects


def test_batch_project_export_v2(
configured_batch_project_with_label: Tuple[Project, Dataset, DataRow,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: We don't need to define all the return types since this is a test. May be cleaner to not have to import all of the types too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd prefer to leave it in case anyone needs to do modification, the person will have full intellisense support, without looking back at the definitions. That's what I always find hard about our python services - we're missing type annotations in a lot of places and the code is hard to reason about without poking around.

@mnoszczak mnoszczak merged commit 19c670f into develop Jun 6, 2023
@mnoszczak mnoszczak deleted the mno/al-5662 branch June 6, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kkim-labelbox kkim-labelbox kkim-labelbox approved these changes

@kopreschko kopreschko Awaiting requested review from kopreschko

@msokoloff1 msokoloff1 Awaiting requested review from msokoloff1

@vbrodsky vbrodsky Awaiting requested review from vbrodsky

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mnoszczak @kkim-labelbox