Skip to content

[PLT-1677] Added get mal import functions to replace old BulkImportRequest class #1909

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

Merged
merged 4 commits into from
Nov 18, 2024

Conversation

Gabefire
Copy link
Collaborator

@Gabefire Gabefire commented Nov 16, 2024

Description

  • Two distinct annotation jobs
    1. Label imports
    2. Prediction imports
  • included get methods for both and delete path for prediction import

Type of change

  • New feature (non-breaking change which adds functionality)

All Submissions

  • Have you followed the guidelines in our Contributing document?
  • Have you provided a description?
  • Are your changes properly formatted?

MALPredictionImport,
)

def get_label_imports(self) -> PaginatedCollection:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note: Not going to add a delete path for regular label imports. This is because the mutation is internal and it can also put the data row in a bad state i.e. label with no annotations. We also already have SDk methods to delete and requeue labels so not needed.

@@ -587,6 +587,20 @@ def parent_id(self) -> str:
"""
return self.project().uid

def delete(self) -> None:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't use the deletable inheritance to make this work for this class due to this being a hard delete, not flipping a field to False.

@Gabefire Gabefire changed the title [PLT-1677] Added get mal import functions [PLT-1677] Added get mal import functions to replace old BulkImportRequest class Nov 16, 2024
@tomislav-peharda
Copy link
Contributor

Im okay approving because of codeowner requirement, but might be useful getting 👀 from someone from Model team

Copy link
Contributor

@apollonin apollonin left a comment

Choose a reason for hiding this comment

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

lgtm

@Gabefire Gabefire merged commit b5b6e38 into develop Nov 18, 2024
17 of 23 checks passed
@Gabefire Gabefire deleted the gu/bulk_import_replacement branch November 18, 2024 20:50
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.

3 participants