-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Improve uv environment support in isaaclab.sh #4269
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?
Conversation
Greptile SummaryThis PR enhances Key improvements:
Critical issues found:
The PR addresses the original issue of incorrect Python versions, but introduces a regression that breaks the "create environment first, install Isaac Sim later" workflow. Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Script as isaaclab.sh
participant UV as uv command
participant Python as Python/Isaac Sim
alt Creating new environment (./isaaclab.sh -u)
User->>Script: ./isaaclab.sh -u my_env
Script->>Script: Check if VIRTUAL_ENV is active
alt Active uv environment detected
Script->>User: [INFO] Detected active uv environment
Script->>User: Use ./isaaclab.sh -i or deactivate first
else No active environment
Script->>Python: is_isaacsim_version_4_5()
Python->>Script: extract_python_exe()
alt Isaac Sim found
Python-->>Script: Return version (4.5 or 5.0+)
Script->>Script: Set python_version (3.10 or 3.11)
Script->>UV: uv venv --python {version} {env_name}
UV-->>Script: Environment created
Script->>User: [INFO] Created environment
else Isaac Sim NOT found
Python-->>Script: ERROR: Unable to find Python
Script->>User: EXIT 1 (BREAKS WORKFLOW)
end
end
end
alt Installing in existing environment (./isaaclab.sh -i)
User->>Script: source my_env/bin/activate
User->>Script: ./isaaclab.sh -i
Script->>Script: extract_python_exe() from VIRTUAL_ENV
Script->>Script: Check pyvenv.cfg for "uv" substring
alt Contains "uv" (substring match)
Script->>Script: Use "uv pip install"
else No "uv" found
Script->>Script: Use "python -m pip install"
end
Script->>User: [INFO] Using environment: {VIRTUAL_ENV}
Script->>Script: Install Isaac Lab extensions
end
|
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.
Additional Comments (4)
-
isaaclab.sh, line 679-685 (link)logic: calling
is_isaacsim_version_4_5requires a working Python executable fromextract_python_exe(). If no environment is active AND_isaac_sim/python.shdoesn't exist AND system python lacksisaacsim-rl, the script will exit with an error before creating the environment. This breaks the workflow for users who want to create a uv environment before installing Isaac Sim.fallback to a default version (3.11) if version detection fails:
-
isaaclab.sh, line 236-237 (link)logic:
grep -q "uv"uses substring matching, which could cause false positives if "uv" appears anywhere inpyvenv.cfg(e.g., in a path like/home/uv_user/or version strings). Use a more specific pattern: -
isaaclab.sh, line 249-250 (link)logic: same substring matching issue as in
extract_pip_command. Use a more specific pattern to avoid false positives: -
isaaclab.sh, line 457-458 (link)logic: same substring matching issue. Use a more specific pattern:
1 file reviewed, 4 comments
- Allow installation into existing uv/venv environments - Auto-detect Isaac Sim version for Python version selection - Display active environment information during installation - Fix tabs command compatibility for non-interactive terminals This change enables more flexible workflow patterns by detecting and using active virtual environments instead of forcing creation of new environments in the IsaacLab directory."
192805a to
f465270
Compare
|
I have fixed the bugs that greptile has mentioned. |
Description
This PR enhances the
isaaclab.shinstallation script to support flexible virtual environment workflows by allowing installation into existing uv/venv environments.Changes
-uwith an active environmentMotivation
Previously, users had to create a new environment in the IsaacLab directory. This PR enables modern Python workflows where users can:
uvproject-based workflowsThe issue is mentioned in [Bug Report] isaaclab.sh does not properly configure python version in install #3783 and Adds optional --python arg when --uv is used to create venv #3560
Type of change
Testing
Tested scenarios:
-uflag./isaaclab.sh -h)Checklist
pre-commitchecks with./isaaclab.sh --format