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

Add support for writing systems in Python #2045

Closed

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Jul 21, 2023

🎉 New feature

Closes #790

Summary

Test it

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

azeey and others added 9 commits July 15, 2023 12:24
Signed-off-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Voldivh <[email protected]>
Signed-off-by: Voldivh <[email protected]>
This allows multiple PythonSystemLoaders to run albeit with a single
python interpreter.

Signed-off-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
@@ -0,0 +1,23 @@
from gz.sim8 import UpdateInfo, EntityComponentManager, Model
Copy link
Contributor

Choose a reason for hiding this comment

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

LICENSE

Comment on lines +20 to +23
<world name="python_system">


<light type="directional" name="sun">
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<world name="python_system">
<light type="directional" name="sun">
<world name="python_system">
<light type="directional" name="sun">

@@ -22,11 +22,15 @@
#include <gz/msgs/server_control.pb.h>
#include <gz/msgs/stringmsg_v.pb.h>


Copy link
Contributor

Choose a reason for hiding this comment

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

remove

Comment on lines +61 to +62
gzerr << "Error while loading module 'sys'\n"
<< _err.what() << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
gzerr << "Error while loading module 'sys'\n"
<< _err.what() << std::endl;
gzerr << "Error while loading module 'sys'\n"
<< _err.what() << std::endl;

Copy link
Contributor

@Voldivh Voldivh left a comment

Choose a reason for hiding this comment

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

Left a couple of minor comments. This looks good! Is it ready to try it out? @azeey

@@ -22,11 +22,15 @@
#include <gz/msgs/server_control.pb.h>
#include <gz/msgs/stringmsg_v.pb.h>


Copy link
Contributor

Choose a reason for hiding this comment

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

Remove?

{
_method(std::forward<Args>(_args)...);
}
catch (const pybind11::error_already_set &_err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
catch (const pybind11::error_already_set &_err)
catch (const py::error_already_set &_err)

@azeey
Copy link
Contributor Author

azeey commented Jul 21, 2023

Left a couple of minor comments. This looks good! Is it ready to try it out? @azeey

Yes, I have an example with some instructions.

// TODO (azeey) Improve formatting.
gzdbg << py::str(searchPaths).cast<std::string>() << std::endl;
}
catch (const pybind11::error_already_set &_err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
catch (const pybind11::error_already_set &_err)
catch (const py::error_already_set &_err)

{
this->pythonModule = py::module_::import(moduleName.c_str());
}
catch (const pybind11::error_already_set &_err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
catch (const pybind11::error_already_set &_err)
catch (const py::error_already_set &_err)

}
this->pythonSystem = getSystem();
}
catch (const pybind11::error_already_set &_err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
catch (const pybind11::error_already_set &_err)
catch (const py::error_already_set &_err)

@Voldivh Voldivh force-pushed the voldivh/python_bindings_model branch from 7bf6366 to 30439ab Compare July 26, 2023 15:29
@azeey azeey added the beta Targeting beta release of upcoming collection label Jul 31, 2023
@Voldivh Voldivh force-pushed the voldivh/python_bindings_model branch 2 times, most recently from 7998bf7 to af115a1 Compare August 8, 2023 15:47
@azeey azeey deleted the branch gazebosim:voldivh/python_bindings_model August 12, 2023 05:39
@azeey azeey closed this Aug 12, 2023
@azeey
Copy link
Contributor Author

azeey commented Aug 17, 2023

It seems that github automatically closed this PR because the branch it was targeting was deleted. I'm also not able to reopen it, so I'll create a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants