Increase code quality and fix test #1124

Daraan · 2024-10-25T17:27:49Z

This change is

Removed usage of pgk_resources and distutils.version.LooseVersion

Ruff E,W, defalts

--ignore F405,F403,F401,RET504,RET505 --exclude tests/ --select E,W,R,PIE,R

minor changes

- minor lint fixes

Daraan · 2024-10-25T17:35:45Z

srunner/osc2/ast_manager/ast_builder.py

-        if scope:
-            pass
-        else:
-            msg = "behavior name: " + name + " is not defined!"
-            pass
-


Daraan · 2024-10-25T17:36:09Z

srunner/osc2/ast_manager/ast_listener.py

-    def enter_variable_declaration(self, node: ast_node.VariableDeclaration):
-        pass
-
-    def exit_variable_declaration(self, node: ast_node.VariableDeclaration):
-        pass
-


double defined

Daraan · 2024-10-25T17:36:23Z

srunner/osc2/ast_manager/ast_node.py

-class VariableDeclaration(Declaration):
-    """
-    'var' fieldName (',' fieldName)* ':' typeDeclarator ('=' (sampleExpression | valueExp) )? NEWLINE;
-    """
-
-    def __init__(self, field_name, field_type):
-        super().__init__()
-        self.field_name = field_name
-        self.field_type = field_type
-        self.set_children(field_name)
-
-    def enter_node(self, listener):
-        if hasattr(listener, "enter_variable_declaration"):
-            listener.enter_variable_declaration(self)
-
-    def exit_node(self, listener):
-        if hasattr(listener, "exit_variable_declaration"):
-            listener.exit_variable_declaration(self)
-
-    def accept(self, visitor):
-        if hasattr(visitor, "visit_variable_declaration"):
-            return visitor.visit_variable_declaration(self)
-        else:
-            return visitor.visit_children(self)
-


Daraan · 2024-10-25T17:36:49Z

srunner/osc2/ast_manager/ast_vistor.py

-    def visit_variable_declaration(self, node: ast_node.VariableDeclaration):
-        return self.visit_children(node)
-


Daraan · 2025-03-14T10:15:27Z

There's still a longer way to go but this bring us a bit closer to our goal.

glopezdiest

Reviewed 28 of 38 files at r1, 26 of 26 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions

glopezdiest

Reviewable status: all files reviewed, 4 unresolved discussions

glopezdiest · 2025-03-27T14:40:26Z

Hey, thanks for the code improvements. I've accepted the PR. If your resolve your comments we can merge it

Daraan

Was about to do so. Thank you for looking into it. I always forget that simple comments do block.

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Daraan)

remove extra lines (carla-simulator#1074) Add misc type default blueprint (carla-simulator#1064) Co-authored-by: Pablo Villanueva-Domingo <[email protected]> Fix: wrong initial transform when using request_new_batch_actors (carla-simulator#1068) (carla-simulator#1075) See: carla-simulator#1068 Co-authored-by: Daraan <[email protected]> Update atomic_trigger_conditions.py (carla-simulator#946) In the StandStill() class, change the EPSILON, 0.001 Implement variable frame rate (carla-simulator#1005) Replaced class variable frame_rate with an argument provided on the CLI. Co-authored-by: glopezdiest <[email protected]> Workflow improvements and code quality passing (carla-simulator#1052) * CI: Checkout v4 and unit tests name correction * Small tweaks to ensure Code Quality passes --------- Co-authored-by: glopezdiest <[email protected]> Fixed metrics wheel parser (carla-simulator#1076) Co-authored-by: glopezdiest <[email protected]> [Feature] Blueprint-based spawn_actor function for CarlaDataProvider (carla-simulator#1071) * blueprint-based spawn_actor function for CDP * Auto-Option to decide tracking * Updated changelog * spawned actors are tracked if they are Vehicle | Walker * Updated spawn_actor description --------- Co-authored-by: glopezdiest <[email protected]> Hotfix: missing AttachmentType in the carla mock module for unittest (carla-simulator#1077) * blueprint-based spawn_actor function for CDP * Auto-Option to decide tracking * Updated changelog * spawned actors are tracked if they are Vehicle | Walker * Updated spawn_actor description * Added missing AttachmentType for carla mock --------- Co-authored-by: glopezdiest <[email protected]> Backward compatible upgrade to fix deprecation warnings concerning LooseVersion and pkg_resources for Python3.8+ (carla-simulator#1058) * upgraded version checking to fix deprecation warnings Removed usage of pgk_resources and distutils.version.LooseVersion * < python3.8 backward compatible version checking * Compacter version checking. --------- Co-authored-by: glopezdiest <[email protected]> Replace deprecated python 3.8 code and add missing __init__.py files for proper python module recognition (carla-simulator#1053) Co-authored-by: glopezdiest <[email protected]> add support for scientific notation parsing while running OpenSCENARIO files (carla-simulator#1023) Co-authored-by: glopezdiest <[email protected]> Update osc2_scenario_configuration.py (carla-simulator#1040) Add super.__init__() to class OSC2ScenarioConfiguration to prevent error "AttributeError: 'OSC2ScenarioConfiguration' object has no attribute 'route'" when running scenario_runner.py with --openscenario2 parameter Co-authored-by: glopezdiest <[email protected]> Remove version redundancy (carla-simulator#1081) Update minimum CARLA version to 0.9.14 (carla-simulator#1082) * Remove version redundancy * Merge branch 'master' of https://github.com/carla-simulator/scenario_runner * Update minimum CARLA version Type-hint comments for CarlaDataProvider (carla-simulator#1066) * Type-hint comments for CarlaDataProvider * Suppress Any import warning * Removed None values, increased type-coverage Added log agent [fix]rename compound_symbol.py (carla-simulator#1089) Fixed a typo in openscenario_parser.py (carla-simulator#1118) * Fixed a typo There was a typo in function "get_traffic_light_from_osc_name", which was accessing "CCarlaDataProvider" insted of "CarlaDataProvider". * Delete the unneeded file Fix: Remove duplicate clean method calls (carla-simulator#1096) * [fix]rename compound_symbol.py * [fix]Remove duplicate clean method calls * Fix Atomic_behaviors AddActor Func No return status Link documentation in README (carla-simulator#1129) Include 0.9.14 carla release as compatible with scenario runner 0.9.13 (carla-simulator#1130) * Link documentation in README * Include 0.9.14 carla release as compatible with scenario runner 0.9.13 * Merge branch 'master' into docs_in_readme Added a missing element to the blackboard (carla-simulator#1141) Co-authored-by: glopezdiest <[email protected]> Replaced deprecated is_intersection with is_junction (carla-simulator#1114) Co-authored-by: glopezdiest <[email protected]> CDP request_new_actor new optional argument. (carla-simulator#1113) spawn_point can be ommited when random_location=True Co-authored-by: glopezdiest <[email protected]> Faster information retrieval if actor key is present (carla-simulator#1098) Co-authored-by: glopezdiest <[email protected]> Don't depend on distutils (carla-simulator#1121) distutils are no longer available in Python 3.12. Therefore, we copy the strtobool function from distutils to our own module. This is in line with https://peps.python.org/pep-0632/#migration-advice. Co-authored-by: glopezdiest <[email protected]> Fix dead actor accumulation in CDP (carla-simulator#1091) * Slightly more informative function Returns bool if actor was in the pool, else otherwise * Fix accumulation of dead actors in CDP actor pool * Dead actor cleaning in CDP._actor_*_maps --------- Co-authored-by: glopezdiest <[email protected]>

This fixes warnings like the one below: .../srunner/scenarios/osc2_scenario.py:515: SyntaxWarning: invalid escape sequence '\W' expression_value = re.split("\W+", option) Co-authored-by: glopezdiest <[email protected]>

also formatting

Daraan · 2025-03-27T18:12:32Z

I've added a few more fixes but ill stop for good now :)

Probably nice would be to just run the formatting tool in an extra PR. And possibly extract other fixes from #1087.

In the long run I think the current tests are way to strict, reducing the rule sets should help.
Possible we could also switch to https://docs.astral.sh/ruff.

glopezdiest

Reviewed 19 of 20 files at r4.
Reviewable status: 56 of 57 files reviewed, all discussions resolved (waiting on @Daraan)

glopezdiest

Reviewed 1 of 20 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Daraan)

glopezdiest

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Daraan)

glopezdiest

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Daraan)

glopezdiest · 2025-03-31T09:02:25Z

Agreed, our code analysis is a bit old and needs a revision. Thanks for the contribution, as always.

Daraan added 14 commits April 12, 2024 14:05

upgraded version checking to fix deprecation warnings

7cbbc67

Removed usage of pgk_resources and distutils.version.LooseVersion

Code cleanup 1

0f185cf

Ruff E,W, defalts

Check return statement

5bd3af8

--ignore F405,F403,F401,RET504,RET505 --exclude tests/ --select E,W,R,PIE,R

flake8-bugbear

2a6c741

remove full width comma

2460452

Simplify code with SIM

6f030d1

remove double defined function

260b4b1

fix too long lines

1c17b5e

Unnecessary lambda and unnecessary else if

9c693c2

parameterless super()

367fc81

minor changes

Whitespace and other E,W fixes

6cfc435

Add docstring to CDP methods

777747e

- minor lint fixes

Avoid unbound Name Error

aeeae56

Solve most atomic behavior warnings

843f371

Daraan force-pushed the code-quality branch from d27b890 to 843f371 Compare October 28, 2024 09:33

Merge branch 'master' into code-quality

8009ef3

Daraan commented Dec 2, 2024

View reviewed changes

Daraan marked this pull request as ready for review March 14, 2025 10:14

Merge branch 'master' into code-quality

1c71f33

glopezdiest approved these changes Mar 27, 2025

View reviewed changes

Daraan commented Mar 27, 2025

View reviewed changes

Daraan and others added 6 commits March 27, 2025 15:44

Merge branch 'master' into code-quality

0bd56c6

Merge branch 'master' into code-quality

7cbe75c

Merge remote-tracking branch 'upstream/master' into code-quality

6f31dd5

osc2_scenario: Fix SyntaxWarnings (carla-simulator#1122)

4d08044

This fixes warnings like the one below: .../srunner/scenarios/osc2_scenario.py:515: SyntaxWarning: invalid escape sequence '\W' expression_value = re.split("\W+", option) Co-authored-by: glopezdiest <[email protected]>

Further formatting

cc35778

Daraan added 9 commits March 27, 2025 18:26

formatting

7e9a5fb

Return iterable

35133a2

fix: missing import, docstring, explicit return

0cb296a

style: unnecessary super

b85a37b

style: import order

5efcb00

fix | ignore further infractions

1262adf

some more formatting

16f61b5

define method in init

4071652

less general Exception

443ec32

also formatting

glopezdiest approved these changes Mar 31, 2025

View reviewed changes

glopezdiest merged commit 46b1198 into carla-simulator:master Mar 31, 2025
1 of 4 checks passed

Daraan deleted the code-quality branch March 31, 2025 09:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Increase code quality and fix test #1124

Increase code quality and fix test #1124

Daraan commented Oct 25, 2024 •

edited by germanros1987

Loading

Daraan Oct 25, 2024

Daraan Oct 25, 2024

Daraan Oct 25, 2024

Daraan Oct 25, 2024

Daraan commented Mar 14, 2025

glopezdiest left a comment

glopezdiest left a comment

glopezdiest commented Mar 27, 2025

Daraan left a comment •

edited

Loading

Daraan commented Mar 27, 2025

glopezdiest left a comment

glopezdiest left a comment

glopezdiest left a comment

glopezdiest left a comment

glopezdiest commented Mar 31, 2025

		def visit_variable_declaration(self, node: ast_node.VariableDeclaration):
		return self.visit_children(node)

Increase code quality and fix test #1124

Increase code quality and fix test #1124

Conversation

Daraan commented Oct 25, 2024 • edited by germanros1987 Loading

Daraan Oct 25, 2024

Choose a reason for hiding this comment

Daraan Oct 25, 2024

Choose a reason for hiding this comment

Daraan Oct 25, 2024

Choose a reason for hiding this comment

Daraan Oct 25, 2024

Choose a reason for hiding this comment

Daraan commented Mar 14, 2025

glopezdiest left a comment

Choose a reason for hiding this comment

glopezdiest left a comment

Choose a reason for hiding this comment

glopezdiest commented Mar 27, 2025

Daraan left a comment • edited Loading

Choose a reason for hiding this comment

Daraan commented Mar 27, 2025

glopezdiest left a comment

Choose a reason for hiding this comment

glopezdiest left a comment

Choose a reason for hiding this comment

glopezdiest left a comment

Choose a reason for hiding this comment

glopezdiest left a comment

Choose a reason for hiding this comment

glopezdiest commented Mar 31, 2025

Daraan commented Oct 25, 2024 •

edited by germanros1987

Loading

Daraan left a comment •

edited

Loading