refactor: flatten sub-module imports to use parent package re-exports#996
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the usability of the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a flattened import structure for the api, engine, and workflow modules, which is a great usability improvement. The use of __getattr__ for lazy loading is well-implemented to avoid unnecessary imports of heavy dependencies. My review includes suggestions to improve maintainability by reducing code duplication, enhance developer experience with better support for static analysis tools, and make the new tests more robust.
There was a problem hiding this comment.
Pull request overview
This PR implements the "Flatten the import structure of areal modules" item from the 2026 Q1 roadmap (#907). It adds __init__.py files that allow commonly used classes to be imported via short paths (e.g., from areal.api import AllocationMode) instead of requiring deep module paths (e.g., from areal.api.alloc_mode import AllocationMode). The implementation uses PEP 562 module-level __getattr__ for lazy imports to avoid eagerly pulling in heavy dependencies like torch.
Changes:
- New
areal/api/__init__.pywith lazy re-exports for engine, scheduler, allocation, and IO types (withAsyncRewardWrapperandRolloutWorkfloweagerly imported since they don't pull in torch) - New
areal/workflow/__init__.pywith lazy re-exports forRLVRWorkflow,MultiTurnWorkflow, andVisionRLVRWorkflow - Modified
areal/engine/__init__.py(previously empty) with lazy re-exports for all four engine classes - New
tests/test_flatten_imports.pywith tests for all new import paths
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
areal/api/__init__.py |
Adds short-path imports for public API types using lazy __getattr__ pattern |
areal/workflow/__init__.py |
Adds short-path imports for all workflow classes using lazy __getattr__ pattern |
areal/engine/__init__.py |
Adds short-path imports for all engine classes (previously empty file) |
tests/test_flatten_imports.py |
Tests the new import paths across all three modules |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fd268da to
7fdd976
Compare
7fdd976 to
6b215a3
Compare
6b215a3 to
d4aa152
Compare
Add __init__.py with lazy re-exports (__getattr__ + __all__) for areal/api, areal/engine, areal/reward, areal/workflow, and __all__ for areal/dataset, then rewrite all external imports across the codebase to use the shorter parent-package form (e.g. `from areal.api import TrainEngine`). Key changes: - All re-exports are fully lazy via __getattr__ (no eager imports) - Flatten ~100 files across areal/, tests/, examples/ - Preserve cli_args deep imports (dozens of config classes, including SchedulingSpec which stays in cli_args) - Preserve intra-package relative imports to avoid circular deps - Reward submodules use sibling-relative imports (from . import ...) - Preserve non-exported symbols (DeviceRuntimeInfo, HttpRequest, etc.)
d4aa152 to
9b1c36d
Compare
…onflicts Runner 2.317.0 does not support node24 required by actions/checkout@v6, causing 'Set up job' failures on dynamically provisioned GCP instances. Key changes: - Bump RUNNER_VERSION from 2.317.0 to 2.332.0 in test-areal.yml and build-docker-image.yml - Remove duplicate imports in test_rollout_controller.py from rebase with PR #996
…onflicts Runner 2.317.0 does not support node24 required by actions/checkout@v6, causing 'Set up job' failures on dynamically provisioned GCP instances. Key changes: - Bump RUNNER_VERSION from 2.317.0 to 2.332.0 in test-areal.yml and build-docker-image.yml - Remove duplicate imports in test_rollout_controller.py from rebase with PR #996
…onflicts Runner 2.317.0 does not support node24 required by actions/checkout@v6, causing 'Set up job' failures on dynamically provisioned GCP instances. Key changes: - Bump RUNNER_VERSION from 2.317.0 to 2.332.0 in test-areal.yml and build-docker-image.yml - Remove duplicate imports in test_rollout_controller.py from rebase with PR #996
…onflicts Runner 2.317.0 does not support node24 required by actions/checkout@v6, causing 'Set up job' failures on dynamically provisioned GCP instances. Key changes: - Bump RUNNER_VERSION from 2.317.0 to 2.332.0 in test-areal.yml and build-docker-image.yml - Remove duplicate imports in test_rollout_controller.py from rebase with PR #996
…onflicts Runner 2.317.0 does not support node24 required by actions/checkout@v6, causing 'Set up job' failures on dynamically provisioned GCP instances. Key changes: - Bump RUNNER_VERSION from 2.317.0 to 2.332.0 in test-areal.yml and build-docker-image.yml - Remove duplicate imports in test_rollout_controller.py from rebase with PR #996
…onflicts Runner 2.317.0 does not support node24 required by actions/checkout@v6, causing 'Set up job' failures on dynamically provisioned GCP instances. Key changes: - Bump RUNNER_VERSION from 2.317.0 to 2.332.0 in test-areal.yml and build-docker-image.yml - Remove duplicate imports in test_rollout_controller.py from rebase with PR #996
…onflicts Runner 2.317.0 does not support node24 required by actions/checkout@v6, causing 'Set up job' failures on dynamically provisioned GCP instances. Key changes: - Bump RUNNER_VERSION from 2.317.0 to 2.332.0 in test-areal.yml and build-docker-image.yml - Remove duplicate imports in test_rollout_controller.py from rebase with PR #996
…onflicts Runner 2.317.0 does not support node24 required by actions/checkout@v6, causing 'Set up job' failures on dynamically provisioned GCP instances. Key changes: - Bump RUNNER_VERSION from 2.317.0 to 2.332.0 in test-areal.yml and build-docker-image.yml - Remove duplicate imports in test_rollout_controller.py from rebase with PR #996
…onflicts Runner 2.317.0 does not support node24 required by actions/checkout@v6, causing 'Set up job' failures on dynamically provisioned GCP instances. Key changes: - Bump RUNNER_VERSION from 2.317.0 to 2.332.0 in test-areal.yml and build-docker-image.yml - Remove duplicate imports in test_rollout_controller.py from rebase with PR #996
…onflicts Runner 2.317.0 does not support node24 required by actions/checkout@v6, causing 'Set up job' failures on dynamically provisioned GCP instances. Key changes: - Bump RUNNER_VERSION from 2.317.0 to 2.332.0 in test-areal.yml and build-docker-image.yml - Remove duplicate imports in test_rollout_controller.py from rebase with PR #996
…onflicts Runner 2.317.0 does not support node24 required by actions/checkout@v6, causing 'Set up job' failures on dynamically provisioned GCP instances. Key changes: - Bump RUNNER_VERSION from 2.317.0 to 2.332.0 in test-areal.yml and build-docker-image.yml - Remove duplicate imports in test_rollout_controller.py from rebase with PR #996
* feat(infra): split Docker image into sglang and vllm variants Build separate sglang and vllm Docker images from a single parameterized Dockerfile. Each variant ships only one inference backend, eliminating dependency conflicts and reducing image size. Key changes: - Dockerfile with ARG BASE_IMAGE/VARIANT for per-variant builds - CI workflows: build-docker-image, test-areal, tag-release, install-test - pyproject.toml: sglang/vllm conflicting extras, cuda renamed to cuda-train - pytest markers and conditional fixtures for backend-specific tests - GRPO tests parametrized over both sglang and vllm inference backends - Docker validation tools auto-detect installed inference variant Refs: #985 * fix(infra): update GCP runner to v2.332.0 and resolve rebase import conflicts Runner 2.317.0 does not support node24 required by actions/checkout@v6, causing 'Set up job' failures on dynamically provisioned GCP instances. Key changes: - Bump RUNNER_VERSION from 2.317.0 to 2.332.0 in test-areal.yml and build-docker-image.yml - Remove duplicate imports in test_rollout_controller.py from rebase with PR #996
…#996) Add __init__.py with lazy re-exports (__getattr__ + __all__) for areal/api, areal/engine, areal/reward, areal/workflow, and __all__ for areal/dataset, then rewrite all external imports across the codebase to use the shorter parent-package form (e.g. `from areal.api import TrainEngine`). Key changes: - All re-exports are fully lazy via __getattr__ (no eager imports) - Flatten ~100 files across areal/, tests/, examples/ - Preserve cli_args deep imports (dozens of config classes, including SchedulingSpec which stays in cli_args) - Preserve intra-package relative imports to avoid circular deps - Reward submodules use sibling-relative imports (from . import ...) - Preserve non-exported symbols (DeviceRuntimeInfo, HttpRequest, etc.) Co-authored-by: Wentai Zhang <zhangwentai.zwt@antgroup.com>
* feat(infra): split Docker image into sglang and vllm variants Build separate sglang and vllm Docker images from a single parameterized Dockerfile. Each variant ships only one inference backend, eliminating dependency conflicts and reducing image size. Key changes: - Dockerfile with ARG BASE_IMAGE/VARIANT for per-variant builds - CI workflows: build-docker-image, test-areal, tag-release, install-test - pyproject.toml: sglang/vllm conflicting extras, cuda renamed to cuda-train - pytest markers and conditional fixtures for backend-specific tests - GRPO tests parametrized over both sglang and vllm inference backends - Docker validation tools auto-detect installed inference variant Refs: #985 * fix(infra): update GCP runner to v2.332.0 and resolve rebase import conflicts Runner 2.317.0 does not support node24 required by actions/checkout@v6, causing 'Set up job' failures on dynamically provisioned GCP instances. Key changes: - Bump RUNNER_VERSION from 2.317.0 to 2.332.0 in test-areal.yml and build-docker-image.yml - Remove duplicate imports in test_rollout_controller.py from rebase with PR #996
Description
Add
__init__.pywith fully lazy re-exports (__getattr__+__all__) forareal/api,areal/engine,areal/reward,areal/dataset, andareal/workflow, then rewrite all external imports across ~100 files to use the shorter parent-package form.For example:
from areal.api.engine_api import TrainEngine→from areal.api import TrainEngine.Related Issue
N/A
Type of Change
Key Changes
__init__.pyre-exports: all symbols use__getattr__— no eager top-level imports, makingareal.apicompletely side-effect free at import timeareal/api(+23 lazy symbols),areal/engine(+11),areal/reward(+3),areal/workflow(+3),areal/dataset(__all__only)areal/trainer/,areal/utils/,areal/models/,areal/engine/,areal/workflow/,areal/infra/,areal/experimental/,tests/,examples/cli_argsdeep imports — dozens of config classes includingSchedulingSpec; explicit path is clearerfrom . import get_math_verify_worker) instead of absolute package imports, eliminating self-import couplingDeviceRuntimeInfo,HttpRequest,HttpGenerationResult, etc.) that remain as deep importsWhat is NOT changed
from areal.api.cli_args import ...(includingSchedulingSpec)from areal.engine.core.*,fsdp_utils.*,megatron_utils.*from areal.api.io_struct import DeviceRuntimeInfo, HttpRequest, ...__all__— internal symbolsareal/api/engine_api.pyimporting fromareal/api/io_struct)Checklist
jb build docs/gemini review)Breaking Change Details (if applicable):
N/A — all old import paths still work. This is a purely additive refactoring.
Additional Context
All re-exports use PEP 562 lazy
__getattr__to ensure zero side effects at module load time. Symbols are only resolved when first accessed, eliminating circular import risks.106 files changed, 425 insertions(+), 265 deletions(-)