Remove unsupported Franka Reorient task - #8168
Conversation
|
run-ci |
There was a problem hiding this comment.
Isaac Lab Review Bot
The removal is internally consistent across registration, configuration, examples, browser data, changelog, and tests, but it deletes two established public entry points without the required deprecation bridge.
- Design and architecture: The registration, config class, example list, browser row, image mapping, and associated test parametrization are removed coherently. The remaining Franka lift configuration structure is unchanged, with no supplied reference still depending on the deleted class internally.
- API: The changed public surface includes the registered Gym ID
Isaac-Reorient-Frankaand the importableFrankaReorientEnvCfgconfig class, including itsplay_mode()behavior. Both are removed immediately, while theIsaac-Lift-Frankaregistration andFrankaLiftEnvCfgentry points remain unchanged. - Compatibility and deprecation: Breaking changes: the
Isaac-Reorient-FrankaGym ID is removed, breakinggym.makeand registry-based config loading, andFrankaReorientEnvCfgis deleted, breaking direct imports and subclasses. Neither old contract remains functional or emits a targeted deprecation warning. The.majorchangelog fragment provides migration guidance but does not satisfy the required deprecation transition. - Implementation: The producer and integration paths were traced through the Franka registration module, config module, heterogeneous-scene example, and generated environment-browser data. The deletion is mechanically complete in the supplied patch; the required correction is to retain functional compatibility shims for the task ID and config class during the deprecation window.
- Style consistency: Module and class docstrings were updated consistently to describe only the remaining lift task. The changelog uses the package fragment structure, and the added test import follows the existing import grouping. No separate style defect was identified.
- Test quality: Only
source/isaaclab_tasks/test/core/test_lift_env_cfg.pychanged. It removes the deleted config from the collision-mesh parametrization and adds a registry-level assertion that the task ID is absent. These edits reflect the proposed immediate removal, but transition coverage will be needed once the required functional deprecation bridge and warning are added.
Significant concerns. Posted 1 actionable finding inline.
Automated review; human maintainers own approval decisions.
| # Register Gym environments. | ||
| ## | ||
|
|
||
| gym.register( |
There was a problem hiding this comment.
🟡 Warning · Compatibility — Public task and config removed without deprecation
Isaac-Reorient-Franka and FrankaReorientEnvCfg are public entry points: a registered gym ID listed in the environment browser and referenced by examples/heterogeneous_scene.py, and an importable configclass. Both vanish in one step, so existing gym.make, config-loading, and import callers fail with no transition. The contribution guide requires a deprecation and migration path for public removals; a .major changelog fragment is not one. Keep both functional with the established deprecation warning for the prescribed window.
|
|
run-ci |
## Description Remove the unsupported `Isaac-Reorient-Franka` task registration, configuration, example, and browser entry. This is the requested breaking removal; downstream users who still need it must keep a local copy or migrate to a maintained reorientation task. This is part 1 of the split of #7829 and is independently reviewable. ## Validation - `test_lift_env_cfg.py`: 12 passed - `uv run --frozen isaaclab -f`: passed - Generated environment browser and changelog checks: passed ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` (cherry picked from commit a1b7e56)
|
Backported to |
Description
Remove the unsupported
Isaac-Reorient-Frankatask registration, configuration, example, and browser entry. This is the requested breaking removal; downstream users who still need it must keep a local copy or migrate to a maintained reorientation task.This is part 1 of the split of #7829 and is independently reviewable.
Validation
test_lift_env_cfg.py: 12 passeduv run --frozen isaaclab -f: passedRelease backport
develop