-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Replace wildcard imports with explicit imports in utils and markers #4362
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?
Replace wildcard imports with explicit imports in utils and markers #4362
Conversation
…odules - Replaces 'from .module import *' with explicit named imports - Improves code readability and IDE autocompletion - Makes it easier to track which symbols are exported - Follows Python best practices for explicit imports Modified files: - source/isaaclab/isaaclab/utils/__init__.py - source/isaaclab/isaaclab/markers/__init__.py Signed-off-by: Anand Kumar Shaw <[email protected]>
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.
Greptile Overview
Greptile Summary
This PR attempts to replace wildcard imports (from module import *) with explicit named imports in the utils and markers modules, following PEP 8 guidelines. However, it contains a critical error:
Critical Issue Found:
The utils module attempts to import four non-existent functions from the mesh submodule:
convert_mesh_to_meshio_meshmake_convex_mesh_from_meshmake_mesh_from_geometrymerge_meshes
These functions are not defined in mesh.py and have never existed in the codebase. The mesh module's __all__ list actually exports only: create_trimesh_from_geom_mesh, create_trimesh_from_geom_shape, convert_faces_to_triangles, and PRIMITIVE_MESH_TYPES.
Positive Notes:
- The markers module replacement is correct - all imported symbols exist and are properly defined
- The approach of listing explicit imports is correct and follows PEP 8
- All other utils module imports appear to be valid (array, buffers, configclass, dict, interpolation, logger, modifiers, string, timer, types, version)
The PR needs to be corrected to import the actual functions exported from the mesh module.
Confidence Score: 0/5
- This PR is NOT safe to merge - it will cause an ImportError at runtime due to non-existent imports from the mesh module.
- The PR introduces a critical syntax/import error that will prevent the utils module from being imported. The four functions being imported from mesh.py do not exist in that module, and this will break any code that tries to import from isaaclab.utils. While the approach and markers changes are correct, this single critical error makes the entire PR non-functional.
- source/isaaclab/isaaclab/utils/init.py requires immediate correction - must replace the non-existent mesh function imports with the actual exports from mesh.py
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| source/isaaclab/isaaclab/utils/init.py | 0/5 | Contains critical import errors - attempts to import 4 non-existent functions from mesh module (convert_mesh_to_meshio_mesh, make_convex_mesh_from_mesh, make_mesh_from_geometry, merge_meshes). This will cause an ImportError at runtime. The PR introduces non-existent symbols that break the utils module import. |
| source/isaaclab/isaaclab/markers/init.py | 5/5 | Correctly replaces wildcard imports with explicit imports. All marker configuration imports and VisualizationMarkers classes are properly listed and exist in the source modules. |
Sequence Diagram
sequenceDiagram
participant User as User Code
participant UtilsInit as utils/__init__.py
participant MeshMod as mesh.py
participant MarkersInit as markers/__init__.py
participant ConfigMod as config/__init__.py
User->>UtilsInit: import isaaclab.utils
UtilsInit->>MeshMod: from .mesh import convert_mesh_to_meshio_mesh, ...
MeshMod-->>UtilsInit: ❌ ImportError - functions don't exist
Note over UtilsInit: Correct imports should be:<br/>from .mesh import create_trimesh_from_geom_mesh, ...<br/>(4 existing functions from __all__)
User->>MarkersInit: import isaaclab.markers
MarkersInit->>ConfigMod: from .config import BLUE_ARROW_X_MARKER_CFG, ...
ConfigMod-->>MarkersInit: ✓ Success - all 11 configs exist
MarkersInit->>User: ✓ Success
- Replace non-existent function names with actual mesh.py exports - Use: create_trimesh_from_geom_mesh, create_trimesh_from_geom_shape - Use: convert_faces_to_triangles, PRIMITIVE_MESH_TYPES - Fix identified by Greptile bot review Signed-off-by: Anand Kumar Shaw <[email protected]>
|
This is my first PR - Landed here after while reading a research paper. Will like to contribute to this community. |
This PR replaces wildcard imports (
from module import *) with explicit named imports in theutilsandmarkersmodules, following PEP 8 guidelines to improve code clarity and maintainability.PEP 8 Guidance on Wildcard Imports
From PEP 8 - Imports: