-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Actuator Config Circular Import Remediation #3994
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?
Actuator Config Circular Import Remediation #3994
Conversation
Greptile OverviewGreptile SummaryThis PR successfully refactors the actuator configuration modules to resolve circular import issues by splitting the monolithic Key changes:
Critical issue found: Confidence Score: 2/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User as External Code
participant Init as actuators/__init__.py
participant BaseCfg as actuator_base_cfg.py
participant PDCfg as actuator_pd_cfg.py
participant NetCfg as actuator_net_cfg.py
participant PD as actuator_pd.py
participant Net as actuator_net.py
participant Base as actuator_base.py
Note over User,Base: Before: Circular Import Issue
Note over User: Old: from actuator_cfg import *
Note over User: (actuator_cfg imported from actuator_pd/net)
Note over User,Base: After: Clean Module Structure
User->>Init: from isaaclab.actuators import ImplicitActuatorCfg
Init->>PDCfg: import from actuator_pd_cfg
PDCfg->>PD: import actuator_pd module
PDCfg->>BaseCfg: import ActuatorBaseCfg
Note over PDCfg: No circular dependency!
User->>Init: from isaaclab.actuators import ActuatorNetLSTMCfg
Init->>NetCfg: import from actuator_net_cfg
NetCfg->>Net: import actuator_net module
NetCfg->>PDCfg: import DCMotorCfg
Note over NetCfg: Clean dependency chain!
Note over Base,PD: TYPE_CHECKING imports prevent runtime circular imports
Base-->>BaseCfg: TYPE_CHECKING: import ActuatorBaseCfg
PD-->>PDCfg: TYPE_CHECKING: import configs
Net-->>NetCfg: TYPE_CHECKING: import configs
|
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.
18 files reviewed, no comments
738c478 to
0dfbf47
Compare
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 (1)
-
source/isaaclab/isaaclab/actuators/actuator_cfg.py, line 1-291 (link)logic: Old
actuator_cfg.pyfile still exists but should be deletedThis file was supposed to be deleted as part of the refactoring to split configs into separate modules. Having both the old monolithic file and the new split files creates:
- Duplicate class definitions (all configs exist in two places)
- Potential for import confusion
- The circular import issue this PR aims to fix could persist through this file
Delete this file since all classes are now properly exported through:
actuator_base_cfg.pyactuator_pd_cfg.pyactuator_net_cfg.py__init__.py
18 files reviewed, 1 comment
Description
This PR breaks the actuator config into separate files in order to avoid a circular import which arises once the configuration is imported into the actuator base class for more than type hinting.
Type of change
.pyfiles and thus modules. They can still be imported directly from theisaaclab.actuatorspackage. External references to these classes through theactuator_cfgmodule will not longer work. Instead, import directly from theisaaclab.actuatorspackage.Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there