-
Notifications
You must be signed in to change notification settings - Fork 109
Address Warp Deprecation Warnings #981
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
base: main
Are you sure you want to change the base?
Address Warp Deprecation Warnings #981
Conversation
Running a Python file with relative imports yields an `ImportError` with `attempted relative import with no known parent package`.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
ac71177 to
dd1e193
Compare
dd1e193 to
419b111
Compare
| from mujoco_warp import test_data | ||
|
|
||
| from . import collision_driver | ||
| from mujoco_warp._src import collision_driver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this change necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only to solve running tests outside of pytest, otherwise the following error happen:
python ./mujoco_warp/_src/broadphase_test.py
Traceback (most recent call last):
File "./mujoco_warp/_src/broadphase_test.py", line 29, in <module>
from . import collision_driver
ImportError: attempted relative import with no known parent package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair - I'm OK with this change.
erikfrey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the cleanup! I think we should revert the changes to pyproject.toml but otherwise LGTM
| "mujoco>=3.3.7", | ||
| "numpy", | ||
| "warp-lang>=1.9.1", | ||
| "warp-lang>=1.11.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need these changes? we should only change this if mjwarp no longer works with >=1.9.1 - is that the case?
| "pygls>=1.0.0,<2.0.0", | ||
| "lsprotocol>=2023.0.1,<2024.0.0", | ||
| "mujoco>=3.3.7.dev0", | ||
| "warp-lang>=1.9.1.dev0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We put this here because when we develop, we want to use a Warp nightly. Please revert this change.
| from mujoco_warp import test_data | ||
|
|
||
| from . import collision_driver | ||
| from mujoco_warp._src import collision_driver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair - I'm OK with this change.
erikfrey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh please also clean up the linter errors. I think you just need to run ruff check --fix.
Following the release of Warp v1.11.0, deprecation warnings were added for symbols meant to be internal. This MR addresses most of these warnings by referencing the public API instead.
Additionally this:
3.4.1.dev851315742since the previously used version doesn't seem available anymore.