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.

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

--input_data_id_prefixに数値を指定しても、TypeErrorが発生しないようにしました。 #154

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

yuji38kwmt
Copy link
Collaborator

@yuji38kwmt yuji38kwmt commented Dec 11, 2024

背景

--input_data_id_prefixに数値を指定すると、TypeErrorが発生しました。

$ poetry run anno3d project upload_scene --project_id ${PROJECT_ID}  --scene_path  tests/resources/kitti3dobj/training/  --upload_kind data --input_data_id_prefix 20241212

  File "/home/yuji/workspace/github.com/annofab-3dpc-editor-cli/anno3d/simple_data_uploader.py", line 177, in upload
    input_data_id_prefix = input_data_id_prefix + "_" if input_data_id_prefix else ""
                           ~~~~~~~~~~~~~~~~~~~~~^~~~~
TypeError: unsupported operand type(s) for +: 'int' and 'str'

app.pyではinput_data_id_prefixの型をstrにしていますが、Fireはint型に変換するためTypeErrorが発生しました。

@staticmethod
    def upload_scene(
        project_id: str,
        scene_path: str,
        input_data_id_prefix: str = "",
        task_id_prefix: str = "", ...):
        pass

google/python-fire#453

対応方法

+演算子を使わずにf-stringで対応しました。
f-stringならinput_data_id_prefixの型がintでもエラーは発生しません。

@yuji38kwmt yuji38kwmt added the bug Something isn't working label Dec 11, 2024
@yuji38kwmt yuji38kwmt changed the title --input_data_id_prefixに数値を指定しても、エラーが発生しないようにしました。 --input_data_id_prefixに数値を指定しても、TypeErrorが発生しないようにしました。 Dec 11, 2024
@yuji38kwmt yuji38kwmt marked this pull request as ready for review December 11, 2024 17:48
@yuji38kwmt yuji38kwmt requested a review from seraphr December 11, 2024 17:49
seraphr
seraphr previously approved these changes Dec 11, 2024
Copy link
Contributor

@seraphr seraphr left a comment

Choose a reason for hiding this comment

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

なるほど… 型情報をもとに動いてるわけじゃないから、そういう事が起こるのか…

@seraphr seraphr dismissed their stale review December 11, 2024 19:17

approveはミスだった

Copy link
Contributor

@seraphr seraphr left a comment

Choose a reason for hiding this comment

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

うーん…
表題の対応のために、このメソッドに手を入れるのは(型安全性が壊れていることを前提にしている&壊れる場所と対処する場所が離れているので)おかしい
が、app.pyでruntime typeチェックをやる場合、なんでほかの引数はチェックしてないの? みたいな問題が発生してしまう。
そのあたりを曖昧にしつつ、問題を解消する手段としてこの変更は有効だろう…

本来的には、型ヒントを元に、型チェックや変換を行ってくれるfireが欲しい話だ…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants