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

Finish adding types to rosidl_parser #832

Open
wants to merge 24 commits into
base: rolling
Choose a base branch
from

Conversation

InvincibleRMC
Copy link
Contributor

Has to add lots of None checks inside the code. So if possible run the CI with downstream to make sure I didn't make a mistake anywhere.

Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
@InvincibleRMC InvincibleRMC marked this pull request as draft October 19, 2024 16:25
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
Signed-off-by: Michael Carlstrom <[email protected]>
@InvincibleRMC InvincibleRMC marked this pull request as ready for review October 19, 2024 19:59
@@ -108,7 +108,7 @@
BooleanValue = Literal['boolean']
OctetValue = Literal['octet']

SignedExplicitIntegerTypeValues = Literal['int8', 'int16', 'int32' 'int64']
SignedExplicitIntegerTypeValues = Literal['int8', 'int16', 'int32', 'int64']
Copy link
Contributor

Choose a reason for hiding this comment

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

Sneaky! Nice catch

if isinstance(t, Token):
if token_type is None or t.type == token_type:
return t
return True
Copy link
Contributor

Choose a reason for hiding this comment

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

This is another nice catch!

@sloretz
Copy link
Contributor

sloretz commented Oct 31, 2024

Pulls: #832
Gist: https://gist.githubusercontent.com/sloretz/d7cadee3ed1582742b70a09693d856be/raw/3a359eb7e3d161765b7fa09b02928204a941bb9e/ros2.repos
BUILD args: --packages-above-and-dependencies rosidl_parser
TEST args: --packages-above rosidl_parser
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14758

  • Linux Build Status (I'm seeing "no space left on device" errors, probably infra issue)
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status


from lark import Lark
from lark.lexer import Token
from lark.tree import Branch
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like RHEL CI is failing with:

10:30:30 Traceback (most recent call last):
10:30:30   File "/home/jenkins-agent/workspace/ci_linux-rhel/ws/install/rosidl_generator_type_description/lib/rosidl_generator_type_description/rosidl_generator_type_description", line 21, in <module>
10:30:30     from rosidl_generator_type_description import generate_type_hash
10:30:30   File "/home/jenkins-agent/workspace/ci_linux-rhel/ws/install/rosidl_generator_type_description/lib/python3.9/site-packages/rosidl_generator_type_description/__init__.py", line 24, in <module>
10:30:30     from rosidl_parser.parser import parse_idl_file
10:30:30   File "/home/jenkins-agent/workspace/ci_linux-rhel/ws/install/rosidl_parser/lib/python3.9/site-packages/rosidl_parser/parser.py", line 30, in <module>
10:30:30     from lark.tree import Branch
10:30:30 ImportError: cannot import name 'Branch' from 'lark.tree' (/usr/lib/python3.9/site-packages/lark/tree.py)

Probably RHEL has a version of Lark that's older than this commit: lark-parser/lark@acf2a26

I suggest making this a try/except ImportError where this file defines Branch if it can't be imported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I added the try/except.

Signed-off-by: Michael Carlstrom <[email protected]>
@sloretz
Copy link
Contributor

sloretz commented Oct 31, 2024

RHEL rebuild: Build Status

Signed-off-by: Michael Carlstrom <[email protected]>
@InvincibleRMC
Copy link
Contributor Author

Moved ParseTree into the try except as well.

@sloretz
Copy link
Contributor

sloretz commented Oct 31, 2024

RHEL rebuild: Build Status

Copy link

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm with green CI. (including a couple of bug fixes!)

Signed-off-by: Michael Carlstrom <[email protected]>
@InvincibleRMC
Copy link
Contributor Author

I removed the try/except and just left the copied the definitions since the Mypy running on Windows and rhel CI does not support try/except yet. Support was added for this in 0.990 python/mypy#13969.

Signed-off-by: Michael Carlstrom <[email protected]>
@sloretz
Copy link
Contributor

sloretz commented Nov 1, 2024

Pulls: #832
Gist: https://gist.githubusercontent.com/sloretz/ea75fed2da465bcae101db4a71a7237e/raw/3a359eb7e3d161765b7fa09b02928204a941bb9e/ros2.repos
BUILD args: --packages-above-and-dependencies rosidl_parser
TEST args: --packages-above rosidl_parser
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14767

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@sloretz
Copy link
Contributor

sloretz commented Nov 1, 2024

Checking latest commit on Windows: Build Status

Edit: test failures:

test 1
    Start 1: mypy

1: Test command: C:\Python38\python.exe "-u" "C:/ci/ws/install/share/ament_cmake_test/cmake/run_test.py" "C:/ci/ws/build/rosidl_parser/test_results/rosidl_parser/mypy.xunit.xml" "--package-name" "rosidl_parser" "--output-file" "C:/ci/ws/build/rosidl_parser/ament_mypy/mypy.txt" "--command" "C:/ci/ws/install/Scripts/ament_mypy.exe" "--xunit-file" "C:/ci/ws/build/rosidl_parser/test_results/rosidl_parser/mypy.xunit.xml"
1: Working Directory: C:/ci/ws/src/InvincibleRMC/rosidl/rosidl_parser
1: Test timeout computed to be: 60
1: -- run_test.py: invoking following command in 'C:\ci\ws\src\InvincibleRMC\rosidl\rosidl_parser':
1:  - C:/ci/ws/install/Scripts/ament_mypy.exe --xunit-file C:/ci/ws/build/rosidl_parser/test_results/rosidl_parser/mypy.xunit.xml
1: 
1: 4 files checked
1: 25 errors
1: rosidl_parser\parser.py:76:40: error: "Tree" expects no type arguments, but 1 given
1: rosidl_parser\parser.py:77:28: error: "Tree" expects no type arguments, but 1 given
1: rosidl_parser\parser.py: note: In function "extract_content_from_ast":
1: rosidl_parser\parser.py:128:48: error: Argument 1 to "scan_values" of "Tree" has incompatible type "Callable[[Union[Token, Tree]], bool]"; expected "Callable[[Union[str, Tree]], bool]"
1: rosidl_parser\parser.py:129:41: error: "str" has no attribute "value"
1: rosidl_parser\parser.py:139:38: error: Argument 1 to "get_const_expr_value" has incompatible type "Union[str, Tree]"; expected "Union[Token, Tree]"
1: rosidl_parser\parser.py:155:43: error: Argument 1 to "get_abstract_type" has incompatible type "Union[str, Tree]"; expected "Union[Token, Tree]"
1: rosidl_parser\parser.py: note: In function "get_first_identifier_value":
1: rosidl_parser\parser.py:323:46: error: Argument 1 to "scan_values" of "Tree" has incompatible type "Callable[[Union[Token, Tree]], bool]"; expected "Callable[[Union[str, Tree]], bool]"
1: rosidl_parser\parser.py:324:12: error: "str" has no attribute "value"
1: rosidl_parser\parser.py: note: In function "get_abstract_type_from_type_spec":
1: rosidl_parser\parser.py:469:30: error: Argument 1 to "get_abstract_type" has incompatible type "Union[str, Tree]"; expected "Union[Token, Tree]"
1: rosidl_parser\parser.py: note: In function "get_abstract_type":
1: rosidl_parser\parser.py:490:50: error: Argument 1 to "scan_values" of "Tree" has incompatible type "Callable[[Union[Token, Tree]], bool]"; expected "Callable[[Union[str, Tree]], bool]"
1: rosidl_parser\parser.py: note: In function "get_positive_int_const":
1: rosidl_parser\parser.py:564:44: error: Argument 1 to "scan_values" of "Tree" has incompatible type "Callable[[Union[Token, Tree]], bool]"; expected "Callable[[Union[str, Tree]], bool]"
1: rosidl_parser\parser.py:569:20: error: "str" has no attribute "value"
1: rosidl_parser\parser.py: note: In function "get_string_literals_value":
1: rosidl_parser\parser.py:702:13: error: Argument 1 to "get_string_literal_value" has incompatible type "Union[str, Tree]"; expected "Union[Token, Tree]"
1: rosidl_parser\parser.py: note: In function "_decode_escape_sequence":
1: rosidl_parser\parser.py:757:12: error: No overload variant of "decode" matches argument types "str", "str"
1: rosidl_parser\parser.py:757:12: note: Possible overload variants:
1: rosidl_parser\parser.py:757:12: note:     def decode(obj: bytes, encoding: Union[Literal['base64'], Literal['base_64'], Literal['base64_codec'], Literal['bz2'], Literal['bz2_codec'], Literal['hex'], Literal['hex_codec'], Literal['quopri'], Literal['quotedprintable'], Literal['quoted_printable'], Literal['quopri_codec'], Literal['uu'], Literal['uu_codec'], Literal['zip'], Literal['zlib'], Literal['zlib_codec']], errors: str = ...) -> bytes
1: rosidl_parser\parser.py:757:12: note:     def decode(obj: str, encoding: Union[Literal['rot13'], Literal['rot_13']] = ..., errors: str = ...) -> str
1: rosidl_parser\parser.py:757:12: note:     def decode(obj: bytes, encoding: str = ..., errors: str = ...) -> str
1: Found 14 errors in 1 file (checked 4 source files)
1: 
1: 
1: Checked files:
1: 
1: * rosidl_parser\__init__.py
1: * rosidl_parser\definition.py
1: * rosidl_parser\parser.py
1: * test\test_parser.py
1: -- run_test.py: return code 1
1: -- run_test.py: verify result file 'C:/ci/ws/build/rosidl_parser/test_results/rosidl_parser/mypy.xunit.xml'
1/1 Test #1: mypy .............................***Failed    7.95 sec

@sloretz
Copy link
Contributor

sloretz commented Nov 4, 2024

@Mergifyio update

Copy link
Contributor

mergify bot commented Nov 4, 2024

update

☑️ Nothing to do

  • #commits-behind > 0 [📌 update requirement]
  • -closed [📌 update requirement]
  • -conflict [📌 update requirement]
  • queue-position = -1 [📌 update requirement]

@sloretz
Copy link
Contributor

sloretz commented Nov 4, 2024

Pulls: #832
Gist: https://gist.githubusercontent.com/sloretz/2c5962b66dd470fe6926c1dbc0f2d439/raw/3a359eb7e3d161765b7fa09b02928204a941bb9e/ros2.repos
BUILD args: --packages-above-and-dependencies rosidl_parser
TEST args: --packages-above rosidl_parser
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14781

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

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