Skip to content

Conversation

@tsuba-neko
Copy link

@tsuba-neko tsuba-neko commented Dec 6, 2023

概要

SDKでリビジョン、バージョンを指定してデータセットを取得できる
以下PRとセット
https://github.com/fastlabel/fastlabel-application/pull/4794

修正内容

  • 単体、複数取得のAPIにオプショナルとしてrevisionのID、versionが指定できるように修正

その他仕様など

以下に記載
https://www.notion.so/fastlabel/SDK-578299c30b0f4152a4c41cdea562685e?pvs=4#c7a31ba0789843f58429048dfbb5925b

@tsuba-neko tsuba-neko changed the base branch from main to develop December 6, 2023 01:01
@tsuba-neko tsuba-neko changed the base branch from develop to main December 6, 2023 01:02
@tsuba-neko tsuba-neko marked this pull request as ready for review December 7, 2023 06:57
@tsuba-neko tsuba-neko requested a review from anegawa-j December 7, 2023 06:57
@tsuba-neko tsuba-neko changed the title Feature/add get dataset param SDKでリビジョン、バージョンを指定してデータセットを取得できる Dec 7, 2023
Copy link
Contributor

Choose a reason for hiding this comment

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

ReadMeにも仕様を追加してもらいたいです!

Copy link
Author

Choose a reason for hiding this comment

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

READMEに追記してみました!

Comment on lines +3921 to +3922
version: str = None,
revision_id: str = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

両方指定されていたらエラーになるのが良いかなと思いましたがいかがでしょうか...?

Copy link
Author

Choose a reason for hiding this comment

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

両方指定の場合はエラーとしました!

Copy link
Contributor

@anegawa-j anegawa-j left a comment

Choose a reason for hiding this comment

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

すみません、1点コメントしました!

self,
dataset: str,
version: str = None,
tags: List[str] = [],
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
tags: List[str] = [],
tags: List[str] = None,

pythonにおいて、mutableなオブジェクトをデフォルト引数に与えると、副作用が発生する可能性があるので、Noneを指定して、関数内で tags = tags or [] という形で初期化する形で記述お願いします!

参考
https://qiita.com/7of9/items/5893e79819613d7f3c17

Copy link
Author

Choose a reason for hiding this comment

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

対応済みです
(非常に勉強になります!!ありがとうございます。)

Copy link
Contributor

@anegawa-j anegawa-j left a comment

Choose a reason for hiding this comment

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

LGTM

@anegawa-j anegawa-j merged commit 2c0f782 into main Dec 8, 2023
@anegawa-j anegawa-j deleted the feature/add-get-dataset-param branch December 8, 2023 05:29
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