Skip to content

Commit

Permalink
Refactor to introduce UploadItem to make ID and other non present mem…
Browse files Browse the repository at this point in the history 
…bers unnecessary at upload
  • Loading branch information
Owen committed Oct 23, 2023
1 parent 3035037 commit cb07c97
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 85 deletions.
35 changes: 14 additions & 21 deletions darwin/future/core/items/uploads.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,20 @@
from typing import Dict, List, Tuple, Union

from darwin.future.core.client import ClientCore
from darwin.future.data_objects.item import Item
from darwin.future.data_objects.item import UploadItem
from darwin.future.data_objects.typing import UnknownType
from darwin.future.exceptions import DarwinException

logger = getLogger(__name__)


async def _build_slots(item: Item) -> List[Dict]:
async def _build_slots(item: UploadItem) -> List[Dict]:
"""
(internal) Builds the slots for an item
Parameters
----------
item: Item
item: UploadItem
The item to build slots for
Returns
Expand Down Expand Up @@ -61,7 +61,7 @@ async def _build_slots(item: Item) -> List[Dict]:
return slots_to_return


async def _build_layout(item: Item) -> Dict:
async def _build_layout(item: UploadItem) -> Dict:
if not item.layout:
return {}

Expand All @@ -83,13 +83,15 @@ async def _build_layout(item: Item) -> Dict:
raise DarwinException(f"Invalid layout version {item.layout.version}")


async def _build_payload_items(items_and_paths: List[Tuple[Item, Path]]) -> List[Dict]:
async def _build_payload_items(
items_and_paths: List[Tuple[UploadItem, Path]]
) -> List[Dict]:
"""
Builds the payload for the items to be registered for upload
Parameters
----------
items_and_paths: List[Tuple[Item, Path]]
items_and_paths: List[Tuple[UploadItem, Path]]
Returns
-------
Expand All @@ -101,19 +103,10 @@ async def _build_payload_items(items_and_paths: List[Tuple[Item, Path]]) -> List
for item, path in items_and_paths:
base_item = {
"name": getattr(item, "name"),
"id": getattr(item, "id"),
"slots": await _build_slots(item),
"dataset_id": getattr(item, "dataset_id"),
"path": str(path),
"processing_status": getattr(item, "processing_status"),
}

if getattr(item, "archived", None):
base_item["archived"] = item.archived

if getattr(item, "priority", None):
base_item["priority"] = item.priority

if getattr(item, "tags", None):
base_item["tags"] = item.tags

Expand All @@ -129,7 +122,7 @@ async def async_register_upload(
api_client: ClientCore,
team_slug: str,
dataset_slug: str,
items_and_paths: Union[Tuple[Item, Path], List[Tuple[Item, Path]]],
items_and_paths: Union[Tuple[UploadItem, Path], List[Tuple[UploadItem, Path]]],
force_tiling: bool = False,
handle_as_slices: bool = False,
ignore_dicom_layout: bool = False,
Expand All @@ -145,7 +138,7 @@ async def async_register_upload(
The slug of the team to register the upload for
dataset_slug: str
The slug of the dataset to register the upload for
items_and_paths: Union[Tuple[Item, Path], List[Tuple[Item, Path]]]
items_and_paths: Union[Tuple[UploadItem, Path], List[Tuple[UploadItem, Path]]]
A list of tuples of Items and Paths to register for upload
force_tiling: bool
Whether to force tiling for the upload
Expand All @@ -158,7 +151,7 @@ async def async_register_upload(
if isinstance(items_and_paths, tuple):
items_and_paths = [items_and_paths]
assert all(
(isinstance(item, Item) and isinstance(path, Path))
(isinstance(item, UploadItem) and isinstance(path, Path))
for item, path in items_and_paths
), "items must be a list of Items"

Expand Down Expand Up @@ -254,7 +247,7 @@ async def async_register_and_create_signed_upload_url(
api_client: ClientCore,
team_slug: str,
dataset_slug: str,
items_and_paths: Union[Tuple[Item, Path], List[Tuple[Item, Path]]],
items_and_paths: Union[Tuple[UploadItem, Path], List[Tuple[UploadItem, Path]]],
force_tiling: bool = False,
handle_as_slices: bool = False,
ignore_dicom_layout: bool = False,
Expand Down Expand Up @@ -350,7 +343,7 @@ def register_upload(
api_client: ClientCore,
team_slug: str,
dataset_slug: str,
items_and_paths: Union[Tuple[Item, Path], List[Tuple[Item, Path]]],
items_and_paths: Union[Tuple[UploadItem, Path], List[Tuple[UploadItem, Path]]],
force_tiling: bool = False,
handle_as_slices: bool = False,
ignore_dicom_layout: bool = False,
Expand Down Expand Up @@ -420,7 +413,7 @@ def register_and_create_signed_upload_url(
api_client: ClientCore,
team_slug: str,
dataset_slug: str,
items_and_paths: Union[List[Tuple[Item, Path]], Tuple[Item, Path]],
items_and_paths: Union[List[Tuple[UploadItem, Path]], Tuple[UploadItem, Path]],
force_tiling: bool = False,
handle_as_slices: bool = False,
ignore_dicom_layout: bool = False,
Expand Down
18 changes: 18 additions & 0 deletions darwin/future/data_objects/item.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,24 @@ def root(cls, values: Dict) -> Dict:
return values


class UploadItem(DefaultDarwin):
# GraphotateWeb.Schemas.DatasetsV2.ItemRegistration.NewItem

# Required fields
name: str
slots: List[ItemSlot] = []

# Optional fields
description: Optional[str] = None
path: str = "/"
tags: Optional[Union[List[str], Dict[str, str]]] = []
layout: Optional[ItemLayout] = None

@validator("name")
def validate_name(cls, v: UnknownType) -> str:
return validate_no_slashes(v)


class Item(DefaultDarwin):
# GraphotateWeb.Schemas.DatasetsV2.ItemRegistration.NewItem

Expand Down
84 changes: 20 additions & 64 deletions darwin/future/tests/core/items/test_upload_items.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@
from pathlib import Path
from typing import Dict, Generator, List, Tuple
from unittest.mock import MagicMock, Mock, patch
from uuid import UUID

import orjson as json
import pytest
import responses

import darwin.future.core.items.uploads as uploads
from darwin.future.core.client import ClientCore, DarwinConfig
from darwin.future.data_objects.item import Item, ItemLayout, ItemSlot
from darwin.future.data_objects.item import ItemLayout, ItemSlot, UploadItem
from darwin.future.exceptions import DarwinException
from darwin.future.tests.core.fixtures import * # noqa: F401,F403

Expand All @@ -20,37 +19,28 @@
class TestBuildSlots:
BUILD_SLOT_RETURN_TYPE = List[Dict]

items_and_expectations: List[Tuple[Item, BUILD_SLOT_RETURN_TYPE]] = []
items_and_expectations: List[Tuple[UploadItem, BUILD_SLOT_RETURN_TYPE]] = []

# Test empty slots
items_and_expectations.append(
(
Item(
name="name_with_no_slots",
id=UUID("00000000-0000-0000-0000-000000000000"),
slots=[],
dataset_id=1,
processing_status="processing",
),
UploadItem(name="name_with_no_slots", slots=[]),
[],
)
)

# Test Simple slot with no non-required fields
items_and_expectations.append(
(
Item(
UploadItem(
name="name_with_simple_slot",
id=UUID("00000000-0000-0000-0000-000000000000"),
slots=[
ItemSlot(
slot_name="slot_name_simple",
file_name="file_name",
storage_key="storage_key",
)
],
dataset_id=1,
processing_status="processing",
),
[
{
Expand All @@ -66,11 +56,8 @@ class TestBuildSlots:
# Test with multiple slots
items_and_expectations.append(
(
Item(
UploadItem(
name="name_with_multiple_slots",
id=UUID("00000000-0000-0000-0000-000000000000"),
dataset_id=1,
processing_status="processing",
slots=[
ItemSlot(
slot_name="slot_name1",
Expand Down Expand Up @@ -104,11 +91,8 @@ class TestBuildSlots:
# Test with `as_frames` optional field
items_and_expectations.append(
(
Item(
UploadItem(
name="name_testing_as_frames",
id=UUID("00000000-0000-0000-0000-000000000000"),
dataset_id=1,
processing_status="processing",
slots=[
ItemSlot(
slot_name="slot_name1",
Expand Down Expand Up @@ -157,11 +141,8 @@ class TestBuildSlots:
# Test with `extract_views` optional field
items_and_expectations.append(
(
Item(
UploadItem(
name="name_testing_extract_views",
id=UUID("00000000-0000-0000-0000-000000000000"),
dataset_id=1,
processing_status="processing",
slots=[
ItemSlot(
slot_name="slot_name1",
Expand Down Expand Up @@ -210,11 +191,8 @@ class TestBuildSlots:
# Test with `fps` semi-optional field - field defaults to 0 if not provided
items_and_expectations.append(
(
Item(
UploadItem(
name="name_with_simple_slot",
id=UUID("00000000-0000-0000-0000-000000000000"),
dataset_id=1,
processing_status="processing",
slots=[
ItemSlot(
slot_name="slot_name25",
Expand Down Expand Up @@ -273,11 +251,8 @@ class TestBuildSlots:
# Test with `metadata` optional field
items_and_expectations.append(
(
Item(
UploadItem(
name="name_with_simple_slot",
id=UUID("00000000-0000-0000-0000-000000000000"),
dataset_id=1,
processing_status="processing",
slots=[
ItemSlot(
slot_name="slot_name",
Expand Down Expand Up @@ -325,11 +300,8 @@ class TestBuildSlots:
# Test with `tags` optional field
items_and_expectations.append(
(
Item(
UploadItem(
name="name_testing_tags",
id=UUID("00000000-0000-0000-0000-000000000000"),
dataset_id=1,
processing_status="processing",
slots=[
ItemSlot(
slot_name="slot_name_with_string_list",
Expand Down Expand Up @@ -365,7 +337,7 @@ class TestBuildSlots:
)

@pytest.mark.parametrize("item,expected", items_and_expectations)
def test_build_slots(self, item: Item, expected: List[Dict]) -> None:
def test_build_slots(self, item: UploadItem, expected: List[Dict]) -> None:
result = asyncio.run(uploads._build_slots(item))
assert result == expected

Expand All @@ -375,12 +347,9 @@ class TestBuildLayout:
"item, expected",
[
(
Item(
UploadItem(
name="test_item",
id=UUID("00000000-0000-0000-0000-000000000000"),
layout=ItemLayout(version=1, type="grid", slots=["slot1", "slot2"]),
dataset_id=1,
processing_status="processing",
),
{
"slots": ["slot1", "slot2"],
Expand All @@ -389,11 +358,8 @@ class TestBuildLayout:
},
),
(
Item(
UploadItem(
name="test_item",
id=UUID("00000000-0000-0000-0000-000000000000"),
dataset_id=1,
processing_status="processing",
layout=ItemLayout(
version=2,
type="grid",
Expand All @@ -410,7 +376,7 @@ class TestBuildLayout:
),
],
)
def test_build_layout(self, item: Item, expected: Dict) -> None:
def test_build_layout(self, item: UploadItem, expected: Dict) -> None:
assert asyncio.run(uploads._build_layout(item)) == expected


Expand All @@ -421,11 +387,8 @@ class TestBuildPayloadItems:
(
[
(
Item(
UploadItem(
name="test_item",
id=UUID("00000000-0000-0000-0000-000000000000"),
dataset_id=1,
processing_status="processing",
slots=[
ItemSlot(
slot_name="slot_name_with_string_list",
Expand All @@ -447,9 +410,6 @@ class TestBuildPayloadItems:
[
{
"name": "test_item",
"id": "00000000-0000-0000-0000-000000000000",
"dataset_id": 1,
"processing_status": "processing",
"path": "test_path",
"slots": [
{
Expand All @@ -473,12 +433,11 @@ class TestBuildPayloadItems:
],
)
def test_build_payload_items(
self, items_and_paths: List[Tuple[Item, Path]], expected: List[Dict]
self, items_and_paths: List[Tuple[UploadItem, Path]], expected: List[Dict]
) -> None:
result = asyncio.run(uploads._build_payload_items(items_and_paths))
result_with_uuid_as_str = [{**item, "id": str(item["id"])} for item in result]

assert result_with_uuid_as_str == expected
assert result == expected


class SetupTests:
Expand All @@ -492,14 +451,11 @@ class TestRegisterUpload(SetupTests):
dataset_slug = "my-dataset"

@pytest.fixture
def items_and_paths(self) -> List[Tuple[Item, Path]]:
def items_and_paths(self) -> List[Tuple[UploadItem, Path]]:
return [
(
Item(
UploadItem(
name="test_item",
id=UUID("00000000-0000-0000-0000-000000000000"),
dataset_id=1,
processing_status="processing",
slots=[
ItemSlot(
slot_name="slot_name_with_string_list",
Expand All @@ -526,7 +482,7 @@ def test_async_register_upload_happy_path(
mock_build_payload_items: MagicMock,
base_client: ClientCore,
default_url: str,
items_and_paths: List[Tuple[Item, Path]],
items_and_paths: List[Tuple[UploadItem, Path]],
) -> None:
items = [
{
Expand Down

0 comments on commit cb07c97

Please sign in to comment.