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.

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

補助情報の種類を「カスタム」ではなく適切な値にする #101

Merged
merged 4 commits into from
Jul 28, 2022
Merged

補助情報の種類を「カスタム」ではなく適切な値にする #101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b6dc17e
SupplementaryDataクラスにdata_typeを追加
pfe171 Jul 19, 2022
434b0cd
補助情報の種類をupload_supplementary関数に渡すようにした
pfe171 Jul 19, 2022
bd06415
linterに対応
pfe171 Jul 19, 2022
fdf614a
supplementary_data_typeのデフォルト引数を除去した
seraphr Jul 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions anno3d/annofab/uploader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from dataclasses import dataclass
from logging import getLogger
from pathlib import Path
from typing import Any, Optional
from typing import Any, Literal, Optional

import boto3
import more_itertools
Expand Down Expand Up @@ -61,12 +61,18 @@ def upload_input_data(self, input_data_id: str, file: Path) -> str:
logger.debug("uploaded input data: %s", input_data)
return data_id

def upload_supplementary(self, input_data_id: str, supplementary_id: str, file: Path) -> str:
def upload_supplementary(
self,
input_data_id: str,
supplementary_id: str,
file: Path,
supplementary_data_type: Literal["custom", "image", "text"],
) -> str:
path = self.upload_tempdata(file)
body = {
"supplementary_data_name": supplementary_id,
"supplementary_data_path": path,
"supplementary_data_type": "custom",
"supplementary_data_type": supplementary_data_type,
"supplementary_data_number": 0,
}
if self._force:
Expand Down
2 changes: 1 addition & 1 deletion anno3d/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def upload_velodyne(
data_id = data_id_prefix + velo_file.name
uploader.upload_input_data(data_id, velo_file)
supp_data = create_frame_meta(tempdir, data_id, 0, sensor_height)
uploader.upload_supplementary(data_id, supp_data.data_id, supp_data.path)
uploader.upload_supplementary(data_id, supp_data.data_id, supp_data.path, supp_data.data_type)
logger.info("uploaded: %s", velo_file)


Expand Down
17 changes: 9 additions & 8 deletions anno3d/simple_data_uploader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import tempfile
from dataclasses import dataclass
from pathlib import Path
from typing import List, Optional, Tuple
from typing import List, Literal, Optional, Tuple

import numpy as np
from scipy.spatial.transform import Rotation
Expand All @@ -32,6 +32,7 @@
class SupplementaryData:
data_id: str
path: Path
data_type: Literal["custom", "image", "text"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

@seraphr annofabapi.models.SupplementaryDataTypeというEnumクラスがありますが、これを利用した方がよいのでしょうか？
それとも、anno3d/simple_data_uploader.pyはannofabapiに依存しない方が良いのでしょうか？

Copy link
Collaborator

Choose a reason for hiding this comment

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

@seraphr と話したこと

  • annofabapi.models.SupplementaryDataTypeを使うメリットはそこまでない
    • annofabapiのAPIを実行する関数で、annofabapi.models.SupplementaryDataType型の値を求めている訳ではな
    • メリットがあるとしたら、SupplementaryDataTypeの値が増えたときに、変更が不要なことぐらい

Copy link
Contributor Author

Choose a reason for hiding this comment

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

すみません、annofabapi.models.SupplementaryDataTypeの存在を完全に見落としていました。

手元で実装を変えて試してみましたが、annofabapi.models.SupplementaryDataTypeを使用しても同じ機能は果たせました。
ですが、個人的にはannofabapi.models.SupplementaryDataTypeを使わない方がコードは見やすく、理解しやすい気がしました。
ただ、annofabapiのドキュメントにはSupplementaryDataTypeとはっきり記載がある点と、仰っていただいたようにSupplementaryDataTypeの値が増えたときのことを考えると、使う方が合理的ともいえると思います。（実際に増えることがあるのかはわかりませんが...）

使った方にしておきたいということでしたら、annofabapi.models.SupplementaryDataTypeを使ったバージョンをコミットしますが、いかがでしょうか？

Copy link
Collaborator

Choose a reason for hiding this comment

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

使った方にしておきたいということでしたら、annofabapi.models.SupplementaryDataTypeを使ったバージョンをコミットしますが、いかがでしょうか？

いろいろ試していただき、ありがとうございます。
曖昧なコメントで申し訳ありません。
@seraphr と話した際、「annofabapi.models.SupplementaryDataTypeを使わない」という結果になったので、対応は不要です。



def create_frame_meta(
Expand All @@ -50,7 +51,7 @@ def create_frame_meta(
with file.open("w", encoding="UTF-8") as writer:
writer.write(meta.to_json(ensure_ascii=False, indent=3))

return SupplementaryData(data_id, file)
return SupplementaryData(data_id, file, "text")


def _create_image_meta(
Expand Down Expand Up @@ -101,7 +102,7 @@ def _create_image_meta(
with file.open("w", encoding="UTF-8") as writer:
writer.write(meta.to_json(ensure_ascii=False, indent=3))

return SupplementaryData(data_id, file)
return SupplementaryData(data_id, file, "text")


def _create_dummy_image_meta(parent_dir: Path, input_data_id: str, number: int) -> SupplementaryData:
Expand All @@ -124,14 +125,14 @@ def _create_dummy_image_meta(parent_dir: Path, input_data_id: str, number: int)
with file.open("w", encoding="UTF-8") as writer:
writer.write(meta.to_json(ensure_ascii=False, indent=3))

return SupplementaryData(data_id, file)
return SupplementaryData(data_id, file, "text")


def _upload_supplementaries(
uploader: Uploader, input_data_id: str, supplementary_list: List[SupplementaryData]
) -> None:
for supp in supplementary_list:
uploader.upload_supplementary(input_data_id, supp.data_id, supp.path)
uploader.upload_supplementary(input_data_id, supp.data_id, supp.path, supp.data_type)


async def upload_async(
Expand Down Expand Up @@ -180,7 +181,7 @@ def upload(
create_camera_horizontal_fov_provider(camera_horizontal_fov, image_paths, fallback_horizontal_fov)
]
for meta in [
SupplementaryData(camera_image_id(input_data_id, i), image_paths.image),
SupplementaryData(camera_image_id(input_data_id, i), image_paths.image, "image"),
_create_image_meta(
tempdir, image_paths.calib, input_data_id, i, image_paths.camera_settings, fov_provider,
),
Expand All @@ -193,7 +194,7 @@ def upload(
for i in range(0, len(dummy_images))
for meta in [
_create_dummy_image_meta(tempdir, input_data_id, i + image_count),
SupplementaryData(camera_image_id(input_data_id, i + image_count), dummy_images[i]),
SupplementaryData(camera_image_id(input_data_id, i + image_count), dummy_images[i], "image"),
]
]

Expand Down Expand Up @@ -246,7 +247,7 @@ def create_kitti_files(
]
for _ in [shutil.copyfile(image.image, image_path)]
for meta in [
SupplementaryData(image_id, image_path),
SupplementaryData(image_id, image_path, "image"),
_create_image_meta(input_data_dir, image.calib, input_data_id, i, image.camera_settings, fov_provider),
]
]
Expand Down