feat(autoware_lanelet2_divider): add lanelet2 divider tool #130

StepTurtle · 2024-10-04T11:33:49Z

Description

Port lanelet2-map-tile-generator to autoware_tools with name autoware_lanelet2_divider.

cc. @ataparlar @YamatoAndo

Tests performed

Notes for reviewers

Interface changes

Effects on system behavior

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

I've confirmed the contribution guidelines.
The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

The PR follows the pull request guidelines.
The PR has been properly tested.
The PR has been reviewed by the code owners.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

There are no open discussions or they are tracked via tickets.
The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

Signed-off-by: Barış Zeren <[email protected]>

wenrir

Some minor comments that might be irrelevant (please ignore if so), thank you for your consideration.

map/autoware_lanelet2_divider/autoware_lanelet2_divider/osmium_tool/osmium_tool.py

map/autoware_lanelet2_divider/autoware_lanelet2_divider/xml_tool/xml_tool.py

map/autoware_lanelet2_divider/autoware_lanelet2_divider/data_preperation/data_preperation.py

map/autoware_lanelet2_divider/autoware_lanelet2_divider/autoware_lanelet2_divider.py

map/autoware_lanelet2_divider/README.md

Signed-off-by: Barış Zeren <[email protected]>

wenrir · 2024-11-07T02:41:07Z

@StepTurtle thank you for considering my review, I'm not able to resolve my own comments so please feel free to resolve/remove them.

map/autoware_lanelet2_divider/autoware_lanelet2_divider/xml_tool/xml_tool.py

YamatoAndo · 2024-11-19T07:47:31Z

map/autoware_lanelet2_divider/autoware_lanelet2_divider/xml_tool/xml_tool.py

+    whole_map_xml = ET.parse(input_osm_file_path)
+    whole_map_root = whole_map_xml.getroot()
+
+    add_version = any(root_element != "version" for root_element in whole_map_root.attrib)


Why is it set to update as long as even one non-version element is found?
In this code, it updates even if there is a version tag, as long as there are other elements present.

If I understand correct, this commit should fix this problem: bbb88cb

YamatoAndo · 2024-11-19T07:49:14Z

map/autoware_lanelet2_divider/autoware_lanelet2_divider/xml_tool/xml_tool.py

+    add_version = any(root_element != "version" for root_element in whole_map_root.attrib)
+
+    if add_version:
+        whole_map_root.set("version", "0.6")


Why is the version fixed to 0.6?
I’d like to know the reason for choosing 0.6.

@ataparlar Do you have any idea on that?

We are adding version tag because osmium tool returns an error if there is missing tag, but I am not sure on the these numbers?

@ataparlar Do you have any idea on that?

YamatoAndo · 2024-11-19T07:54:48Z

map/autoware_lanelet2_divider/autoware_lanelet2_divider/xml_tool/xml_tool.py

+                    add_version = False
+                    break
+            if add_version:
+                element.set("version", "1")


Why is the version fixed to 1?
And I’d like to understand the reason behind adding the version tag to each element. Why?

YamatoAndo · 2024-11-19T07:57:25Z

map/autoware_lanelet2_divider/autoware_lanelet2_divider/xml_tool/xml_tool.py

+            if add_version:
+                element.set("version", "1")
+
+    whole_map_xml.write(input_osm_file_path, encoding="utf-8", xml_declaration=True)


Please avoid directly modifying the input file.
Could you revise the implementation to create a new temporary file instead?

It creates a temp osm and remove end of the tool.

related commit: 574564a

YamatoAndo · 2024-11-20T04:21:27Z

Please fix spell-check.

Thank you!

Signed-off-by: Barış Zeren <[email protected]>

…oware_tools into feat/add_lanelet2_divider

Signed-off-by: Barış Zeren <[email protected]>

StepTurtle · 2024-11-21T22:29:31Z

Please fix spell-check.

Thank you!

Only one spelling error left in requirements.txt, not sure how can we ignore it.

YamatoAndo · 2024-11-25T08:49:15Z

Only one spelling error left in requirements.txt, not sure how can we ignore it.

The dictionary is managed in https://github.com/tier4/autoware-spell-check-dict.
If necessary, please submit a PR to the repository to add words.

Please note that this repository might be migrated to another one (https://github.com/autowarefoundation/autoware-spell-check-dict) in the near future.

StepTurtle · 2024-11-27T02:52:35Z

Only one spelling error left in requirements.txt, not sure how can we ignore it.

The dictionary is managed in https://github.com/tier4/autoware-spell-check-dict. If necessary, please submit a PR to the repository to add words.

Please note that this repository might be migrated to another one (https://github.com/autowarefoundation/autoware-spell-check-dict) in the near future.

I think it is not really necessary, if it does not block the PR. Otherwise I wanna ask you to add this word with easy-to-add-a-word-using-workflow, because I cannot use it this repo under TIER IV organization.

YamatoAndo · 2024-11-27T07:16:56Z

I think it is not really necessary, if it does not block the PR.

"spell-check-partial" is a mandatory CI check,
so you must pass "spell-check-partial" to merge the PR.

I cannot use it this repo under TIER IV organization.

Sorry about that. I’ve submitted the PR (tier4/autoware-spell-check-dict#833) on your behalf.
Are there any other words you’d like me to add?

StepTurtle · 2024-11-27T14:54:53Z

Are there any other words you’d like me to add?

gdal is enough for now, thanks.

feat: add lanelet2 divider tool

3cbbf6e

Signed-off-by: Barış Zeren <[email protected]>

StepTurtle assigned ataparlar and StepTurtle Oct 4, 2024

github-actions bot added type:documentation component:map labels Oct 4, 2024

style(pre-commit): autofix

99ec770

StepTurtle mentioned this pull request Oct 4, 2024

Add lanelet2 divider tool #131

Open

5 tasks

YamatoAndo self-requested a review October 7, 2024 04:22

wenrir reviewed Oct 28, 2024

View reviewed changes

StepTurtle and others added 15 commits October 28, 2024 15:48

fix: wrong maintainer format

cc7ffbf

Signed-off-by: Barış Zeren <[email protected]>

fix: add missing output directory check

03005a9

Signed-off-by: Barış Zeren <[email protected]>

chore: add args to osmium tool

a537e1e

Signed-off-by: Barış Zeren <[email protected]>

chore: remove unnecessary else

56f3dc5

Signed-off-by: Barış Zeren <[email protected]>

refactor: extract_osm_file function

926788f

Signed-off-by: Barış Zeren <[email protected]>

docs: add doxygen-style comments

c89bc27

Signed-off-by: Barış Zeren <[email protected]>

chore: remove redundant else

f1fc2de

Signed-off-by: Barış Zeren <[email protected]>

refactor: missing version fuction

c5971df

Signed-off-by: Barış Zeren <[email protected]>

refactor: loops in complete_missing_elements function

d74b05a

Signed-off-by: Barış Zeren <[email protected]>

refactor: data_preparation function

b714afc

Signed-off-by: Barış Zeren <[email protected]>

refactor: print functions with Debug

bc46f7f

Signed-off-by: Barış Zeren <[email protected]>

fix: spelling errors

262b9d6

Signed-off-by: Barış Zeren <[email protected]>

style(pre-commit): autofix

6af5e19

fix: pre-commit.ci

5ae8394

Signed-off-by: Barış Zeren <[email protected]>

style(pre-commit): autofix

3cbcf70

YamatoAndo reviewed Nov 19, 2024

View reviewed changes

map/autoware_lanelet2_divider/autoware_lanelet2_divider/xml_tool/xml_tool.py Show resolved Hide resolved

YamatoAndo reviewed Nov 19, 2024

View reviewed changes

StepTurtle and others added 9 commits November 21, 2024 22:49

chore: create temp osm

574564a

Signed-off-by: Barış Zeren <[email protected]>

chore: update version function

bbb88cb

Signed-off-by: Barış Zeren <[email protected]>

style(pre-commit): autofix

ae49921

fix: spell check

3383834

Signed-off-by: Barış Zeren <[email protected]>

Merge branch 'feat/add_lanelet2_divider' of github.com:StepTurtle/aut…

4389d6b

…oware_tools into feat/add_lanelet2_divider

style(pre-commit): autofix

0d0ccea

fix: spell check

27e4fe1

Signed-off-by: Barış Zeren <[email protected]>

style(pre-commit): autofix

d6975ae

fix: spell check

40e4076

Signed-off-by: Barış Zeren <[email protected]>

YamatoAndo mentioned this pull request Nov 27, 2024

feat(words): add gdal tier4/autoware-spell-check-dict#833

Merged

Merge branch 'main' into feat/add_lanelet2_divider

4ed7577

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(autoware_lanelet2_divider): add lanelet2 divider tool #130

feat(autoware_lanelet2_divider): add lanelet2 divider tool #130

StepTurtle commented Oct 4, 2024

wenrir left a comment

wenrir commented Nov 7, 2024

YamatoAndo Nov 19, 2024

StepTurtle Nov 21, 2024

YamatoAndo Nov 19, 2024

StepTurtle Nov 21, 2024

StepTurtle Dec 5, 2024

YamatoAndo Nov 19, 2024

YamatoAndo Nov 19, 2024

StepTurtle Nov 21, 2024

YamatoAndo commented Nov 20, 2024

StepTurtle commented Nov 21, 2024

YamatoAndo commented Nov 25, 2024

StepTurtle commented Nov 27, 2024

YamatoAndo commented Nov 27, 2024

StepTurtle commented Nov 27, 2024

feat(autoware_lanelet2_divider): add lanelet2 divider tool #130

Are you sure you want to change the base?

feat(autoware_lanelet2_divider): add lanelet2 divider tool #130

Conversation

StepTurtle commented Oct 4, 2024

Description

Related links

Tests performed

Notes for reviewers

Interface changes

Effects on system behavior

Pre-review checklist for the PR author

In-review checklist for the PR reviewers

Post-review checklist for the PR author

wenrir left a comment

Choose a reason for hiding this comment

wenrir commented Nov 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YamatoAndo commented Nov 20, 2024

StepTurtle commented Nov 21, 2024

YamatoAndo commented Nov 25, 2024

StepTurtle commented Nov 27, 2024

YamatoAndo commented Nov 27, 2024

StepTurtle commented Nov 27, 2024