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

Pass modules to PythonExpression #655

Merged
merged 13 commits into from
Dec 21, 2022

Conversation

RobertBlakeAnderson
Copy link
Contributor

Added ability to pass Python modules to the PythonExpression substitution.
Allows eval of more expressions.

For example, my motivating use case was:
robot_desc = PythonExpression(["process_file('", filepath, "').toprettyxml(indent=' ')"], xacro.__dict__)

@RobertBlakeAnderson
Copy link
Contributor Author

Any feedback on this?

@methylDragon
Copy link
Contributor

Could you add tests?
@RobertBlakeAnderson

@RobertBlakeAnderson
Copy link
Contributor Author

Sure thing! I'll get to it shortly.

@RobertBlakeAnderson
Copy link
Contributor Author

Done. I also fixed a couple tiny issues that I found while creating and running the tests.

@methylDragon
Copy link
Contributor

methylDragon commented Dec 14, 2022

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

@RobertBlakeAnderson
Copy link
Contributor Author

Got it working with Substitutions in the constructor. Updated tests and added new ones for the XML. I'll now look into the matter of keying the definitions to their modules, with backwards compatibility. As for naming, maybe python_modules or python_deps would be less ambiguous.

…tion.

Allows eval of more expressions.
Signed-off-by: Blake Anderson

Signed-off-by: Blake Anderson <blakeanderson@utexas.edu>
Signed-off-by: Blake Anderson <blakeanderson@utexas.edu>
It is now easier to pass multiple modules.
Also simpler syntax.

Signed-off-by: Blake Anderson <blakeanderson@utexas.edu>
Signed-off-by: Blake Anderson <blakeanderson@utexas.edu>
Signed-off-by: Blake Anderson <blakeanderson@utexas.edu>
Looks like a copy/paste error from another test.

Signed-off-by: Blake Anderson <blakeanderson@utexas.edu>
Signed-off-by: Blake Anderson <blakeanderson@utexas.edu>
Add more tests for describe()

Signed-off-by: Blake Anderson <blakeanderson@utexas.edu>
Signed-off-by: Blake Anderson <blakeanderson@utexas.edu>
Definitions from modules must be prepended by the module name.
Ex: 'sys.getrefcount' instead of just 'getrefcount'.
The math module is an exception for backwards compatibility.

Signed-off-by: Blake Anderson <blakeanderson@utexas.edu>
Signed-off-by: Blake Anderson <blakeanderson@utexas.edu>
@RobertBlakeAnderson
Copy link
Contributor Author

Done. The expressions will now require the module names before the definitions you use. For backwards compatibility, math definitions will work with or without the module name, and the tests include cases for both. If you wish, the implicit references could be considered deprecated for removal at a future date.

I think this PR is ready to go now.

@methylDragon
Copy link
Contributor

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

🔥

Signed-off-by: Blake Anderson <blakeanderson@utexas.edu>
@methylDragon
Copy link
Contributor

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

@methylDragon
Copy link
Contributor

Flake 8 issues :o

https://ci.ros2.org/job/ci_linux-aarch64/12418/testReport/launch.test/test_flake8/test_flake8/

I think just change the name locals

Signed-off-by: Blake Anderson <blakeanderson@utexas.edu>
@methylDragon
Copy link
Contributor

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

@methylDragon methylDragon merged commit b129eb6 into ros2:rolling Dec 21, 2022
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