Skip to content

Remove star imports - #4983

Open
jasonb5 wants to merge 5 commits into
masterfrom
fix/refactor-star-imports
Open

Remove star imports#4983
jasonb5 wants to merge 5 commits into
masterfrom
fix/refactor-star-imports

Conversation

@jasonb5

@jasonb5 jasonb5 commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Description

Removes wildcard (from ... import *) imports across the CIME Python codebase
and applies consistent PEP 8 import ordering to every file touched. Wildcard
imports hide a module's true dependencies, defeat static analysis, and let
unrelated symbols collide across modules.

Star imports removed (106 files)

  • 97 files — replaced from CIME.XML.standard_module_setup import * with
    explicit imports
  • 18 files — replaced from CIME.test_status import * with explicit imports
  • 9 files appear in both groups
  • from CIME.Tools.standard_script_setup import * in CIME/scripts/* is
    intentionally left in place (that module is the script bootstrap shim).

Import reordering (114 additional files)

Where imports were already explicit, the order was normalized to PEP 8 style:

  1. import a, b, c collapsed to one module per line
  2. Three groups separated by blank lines: stdlib → third-party → first-party (CIME)
  3. Each group alphabetized
  4. Symbols inside from X import (a, b, c) alphabetized

Backward compatibility preserved

  • CIME/XML/standard_module_setup.py — marked DEPRECATED in the docstring;
    explicit imports tagged # noqa: F401 so external code that still does
    from CIME.XML.standard_module_setup import * (E3SM, CESM, NorESM
    cime_config/) continues to receive the same names.
  • CIME/Tools/standard_script_setup.py — same treatment; also adds explicit
    re-exports of argparse and logging.

Totals

  • 220 files changed
  • +1,327 / −841 lines
  • No tooling/config changes (no pyproject.toml, setup.cfg, or .isort.cfg
    modified)

Test status

Running pytest CIME/tests/ -k test_unit (with CIME_MACHINE=linux-generic,
E3SM cime_config/) yields 11 failed, 378 passed, 13 skipped. The same
11 failures reproduce identically on master and on the base branch
feat/refactor-core-foundation (PR #4982). They are pre-existing issues
unrelated to this PR
— caused by E3SM machine-config / baseline mismatches
in the local test environment, not by import changes. Confirmed by running
the same suite on all three branches from clean worktrees.

Depends on: #4982

Checklist

  • My code follows the style guidelines of this project (black formatting)
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • I have added tests that exercise my feature/fix and existing tests continue to pass
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding additions and changes to the documentation

@github-actions

Copy link
Copy Markdown
Contributor

@jasonb5
jasonb5 changed the base branch from master to feat/refactor-core-foundation June 18, 2026 20:21
Base automatically changed from feat/refactor-core-foundation to master July 8, 2026 17:38
@jasonb5
jasonb5 force-pushed the fix/refactor-star-imports branch from cafef9e to 83a10fb Compare July 16, 2026 22:50
@jasonb5
jasonb5 marked this pull request as ready for review July 22, 2026 15:21
@jasonb5
jasonb5 requested a review from Copilot July 22, 2026 15:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes wildcard (from ... import *) imports across the CIME Python codebase and normalizes import blocks to consistent PEP 8 ordering. It also preserves downstream compatibility by keeping the legacy “standard_*_setup” modules as deprecated re-export shims.

Changes:

  • Replaced star imports (notably CIME.XML.standard_module_setup and CIME.test_status) with explicit imports across many modules.
  • Added/updated deprecation + compatibility re-exports in CIME/XML/standard_module_setup.py and CIME/Tools/standard_script_setup.py.
  • Per-file import ordering cleanup (stdlib/third-party/first-party grouping, one-import-per-line, alphabetization).

Reviewed changes

Copilot reviewed 220 out of 220 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
CIME/Tools/list_e3sm_tests Removed standard_script_setup bootstrap but still imports CIME.*; likely breaks standalone execution unless sys.path is bootstrapped.
CIME/Tools/cs.status Same bootstrap issue as above: script imports CIME.* without ensuring repo root is on sys.path.
CIME/Tools/wait_for_tests Same bootstrap issue as above: script imports CIME.* without ensuring repo root is on sys.path.
CIME/Tools/mvsource Removes standard_script_setup bootstrap (and introduces a no-op pass), likely breaking CIME.* imports for standalone execution.
CIME/Tools/standard_script_setup.py Marked deprecated for star imports; retains bootstrap side effects and compatibility re-exports.
CIME/XML/standard_module_setup.py Marked deprecated; keeps exports via explicit imports with # noqa: F401 for downstream star-import compatibility.
CIME/test_scheduler.py Large import cleanup to explicit symbols and ordered groups.
CIME/test_status.py Removed star import; now explicitly imports expect and stdlib modules.
CIME/utils.py Import ordering and one-import-per-line normalization.
CIME/case/case.py Major import block rewritten to explicit imports and reordered groups.
CIME/case/case_setup.py Import block rewritten to explicit imports and reordered groups.
CIME/case/case_run.py Import block rewritten to explicit imports and reordered groups.
CIME/SystemTests/system_tests_common.py Import block rewritten to explicit imports and reordered groups.
CIME/XML/generic_xml.py Import block rewritten; small formatting adjustments around expect(...) calls.
CIME/XML/files.py Import block rewritten and reordered.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 8 to +12
import argparse
import logging


from CIME import get_tests, utils
Comment thread CIME/Tools/cs.status
Comment on lines +13 to 17
import os
import sys


from CIME import test_status
Comment thread CIME/Tools/wait_for_tests
Comment on lines +12 to 16
import os
import sys


import CIME.wait_for_tests
Comment thread CIME/Tools/mvsource
Comment on lines 15 to 19
try:
from standard_script_setup import *
pass

from CIME.case import Case
from CIME.utils import symlink_force
@jasonb5
jasonb5 force-pushed the fix/refactor-star-imports branch from 83a10fb to 3bef0df Compare July 22, 2026 17:11
jasonb5 added 3 commits July 22, 2026 10:28
Restores the sys.path bootstrap guarantee that was previously provided
as a side-effect of 'from CIME.XML.standard_module_setup import *'.
That star-import was removed from ~200 internal modules in the
standard_module_setup refactor; without this guard, external consumers
(e.g. downstream cime_config code) that relied on importing any
CIME.XML.* module to get the repo root on sys.path would silently break.

Using append (not insert) matches the legacy standard_module_setup.py:18
semantics and avoids shadowing caller-controlled sys.path entries.
…_setup parity

Adds bootstrap_cime_script() as a full-fidelity drop-in replacement for
'from standard_script_setup import *' in entry-point scripts.  It
performs the dual Python version check (3.9 hard, 3.10 warn), calls
bootstrap_cime() for sys.path/CIMEROOT setup, and enables unbuffered
I/O via CIME.utils.stop_buffering_output().

stop_buffering_output is imported lazily to preserve the standalone
nature of the bootstrap module — it must remain importable before the
CIME package is fully on sys.path.  Also expands the module docstring to
document the public API surface and migration intent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants