Handle jointless-articulation warnings in USD tests - #3124
Conversation
OpenUSD < 26.0 -- the usd-exchange build resolved on aarch64 -- reports
an articulation root that has no joints as an invalid ArticulationDesc,
so parse_usd emits a UserWarning; usd-core >= 26.0 treats it as valid.
Several test_import_usd fixtures intentionally import single-body
(jointless) articulations, which Newton parses identically either way.
This made them fail on aarch64 only once the suite escalates newton.*
warnings to errors.
Drop the unnecessary ArticulationRootAPI from the parsing fixtures that
only used it as scaffolding around a single body. For the composition
and parent-offset fixtures, where the articulation is the behavior under
test, assert exactly the expected warning on OpenUSD < 26.0 while leaving
every other warning subject to the ambient policy, so an unexpected
warning still surfaces.
Also tighten test_contact_response_legacy_shape_fallback: replace the
blanket simplefilter("always") with a DeprecationWarning-scoped filter
and exact-match the per-attribute migration messages.
📝 WalkthroughWalkthroughThis PR updates ChangesUSD Warning and Articulation Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
newton/tests/test_import_usd.py (1)
38-39: ⚡ Quick winAvoid a Sphinx
:func:link to privateparse_usd.Line 38 references
:func:\~newton.utils.parse_usd`, butparse_usdis a private helper and not a stable public Sphinx target. Prefer inline code (``parse_usd()``) or reference the public entry point (ModelBuilder.add_usd`).Based on learnings:
parse_usdinnewton/_src/utils/import_usd.pyis private and should not be referenced via Sphinx:func:targets; use inline code literals instead. As per coding guidelines, use Sphinx cross-references against public API paths.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@newton/tests/test_import_usd.py` around lines 38 - 39, Replace the Sphinx :func: cross-reference to the private helper parse_usd with an inline code literal or a public API reference: change ":func:`~newton.utils.parse_usd`" to "``parse_usd()``" or to the public entry point ":meth:`ModelBuilder.add_usd`" so the docs do not point to the private function parse_usd (the helper in import_usd.py) and instead reference a stable public target.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@newton/tests/test_import_usd.py`:
- Around line 38-39: Replace the Sphinx :func: cross-reference to the private
helper parse_usd with an inline code literal or a public API reference: change
":func:`~newton.utils.parse_usd`" to "``parse_usd()``" or to the public entry
point ":meth:`ModelBuilder.add_usd`" so the docs do not point to the private
function parse_usd (the helper in import_usd.py) and instead reference a stable
public target.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 664d4233-9eda-4859-ae2b-ef5c99f3cf8c
📒 Files selected for processing (1)
newton/tests/test_import_usd.py
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Description
test_import_usdhas several fixtures that intentionally import single-body (jointless) articulations. OpenUSD < 26.0 — theusd-exchangebuild resolved onaarch64— reports an articulation root with no joints as an invalidArticulationDesc, soparse_usdemits aUserWarning; usd-core >= 26.0 (every other platform) treats it as valid. Newton imports the body identically either way, so these tests pass today, but they fail onaarch64only once the suite escalatesnewton.*warnings to errors (the upcoming--strict-warningsmode in #3044).Changes:
ArticulationRootAPIfrom the parsing fixtures that only used it as scaffolding around a single body (material / contact response / gap / margin / solref / mujoco-options). They do not test articulations.Usd.GetVersion()), while leaving every other warning subject to the ambient policy so an unexpectednewton.*warning still fails under strict mode.test_contact_response_legacy_shape_fallback: replace the blanketsimplefilter("always"), which masked every warning, with aDeprecationWarning-scoped filter, and exact-match the per-attribute migration messages instead of substring-matching.This is preparatory cleanup with no behavior change on
main(the warning is non-fatal without--strict-warnings), so #3044 can go green onaarch64.Checklist
CHANGELOG.mdhas been updated (if user-facing change) — not applicable, test-onlyTest plan
Verified on both USD builds by simulating #3044's strict filters (
filterwarnings("error", category=DeprecationWarning)andfilterwarnings("error", module=r"newton(\.|$)")): on usd-core 26.0+ the affected tests pass and the_expect_*decorator is a no-op; on the usd-exchange (OpenUSD 25.5) build the parsing fixtures emit no warning, the decorated fixtures record and assert exactly the expected warning, and an unrelated warning still escalates.Summary by CodeRabbit