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

fix(perception): remove typing_extensions #277

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

ktro2828
Copy link
Contributor

Types of PR

  • Bugfix

Description

This PR removes typing_extensions because it can't be imported in WebAuto, like following log

[perception_evaluator_node.py-71] Traceback (most recent call last):
[perception_evaluator_node.py-71]   File "/home/autoware/autoware.proj/install/driving_log_replayer/lib/driving_log_replayer/perception_evaluator_node.py", line 44, in <module>
[perception_evaluator_node.py-71]     from driving_log_replayer.criteria import PerceptionCriteria
[perception_evaluator_node.py-71]   File "/home/autoware/autoware.proj/install/driving_log_replayer/local/lib/python3.10/dist-packages/driving_log_replayer/criteria/__init__.py", line 15, in <module>
[perception_evaluator_node.py-71]     from .perception import CriteriaLevel
[perception_evaluator_node.py-71]   File "/home/autoware/autoware.proj/install/driving_log_replayer/local/lib/python3.10/dist-packages/driving_log_replayer/criteria/perception.py", line 24, in <module>
[perception_evaluator_node.py-71]     from typing_extensions import Self
[perception_evaluator_node.py-71] ModuleNotFoundError: No module named 'typing_extensions'

@hayato-m126 Actually, we have two options

    1. remove typing_extensions
    1. add python3-typing-extensions to package.xml, note that it can be insatalled with rosdep

How to review this PR

Others

@ktro2828 ktro2828 changed the title fix(perception): remove typing_extensions fix(perception): remove typing_extensions Oct 11, 2023
@ktro2828 ktro2828 requested a review from hayato-m126 October 11, 2023 12:28
@ktro2828 ktro2828 marked this pull request as ready for review October 11, 2023 12:29
Copy link
Collaborator

@hayato-m126 hayato-m126 left a comment

Choose a reason for hiding this comment

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

Thanks.
LGTM

@hayato-m126
Copy link
Collaborator

hayato-m126 commented Oct 11, 2023

It is better to add as few extra dependencies as possible for maintenance convenience.
So I think it is better to import from __future__ like this PR

@ktro2828
Copy link
Contributor Author

@hayato-m126 OK, I will test this PR with WebAuto first. and after the test works well, I will merge this PR.

@ktro2828
Copy link
Contributor Author

ktro2828 commented Oct 12, 2023

I'm running the test here TIER IV INTERNAL LINK

@ktro2828
Copy link
Contributor Author

@hayato-m126 I confirmed the test works well TIER IV INTERNAL LINK

@hayato-m126 hayato-m126 merged commit 6d147ab into develop Oct 12, 2023
@hayato-m126 hayato-m126 deleted the fix/typing-extensions branch October 12, 2023 05:45
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.

2 participants