ENH: handle nested and star imports

[TTsangSC]

(See earlier and more discussion at: #438, TTsangSC#6)

Motivation

The current line_profiler.autoprofile implementation doesn't allow for handling imports nested inside code blocks, unless --prof-mod=<executed/script/or/module> --prof-imports is used:

try:
    from foo import foobar  # Not profiled even with `--prof-mod=foo.foobar`
except ImportError:
    from bar import foobar  # Not profiled even with `--prof-mod=bar.foobar`

However, the use of --prof-imports would also mean that all imported names will be indiscriminately profiled; thus we either lose the fine-grained control that --prof-mod=some.specific.target gives us, or the ability to capture nested imports like the ones found above.

Plus, star-imports are still not handled (#434 mitigated the crash, but did not add actual profiling for them yet):

# `spam.py`:
#   __all__ = ('ham', 'eggs')
#
#   ham = 'SOME_DATA'
#
#
#   def eggs() -> None:
#       ...

from spam import *  # Not profiled even with `--prof-mod=spam`
...

Changes (brief summary)

This draft PR remedies both by:

• Updating ProfmodExtractor to do an AST pass for imports, not just scanning over ast.Module.body.
• Adding ast_profile_transformer.py::ast_create_star_import_node() and line_profiler_utils.py::add_star_import() to handle the post-hoc profiling of star-imported names.
• Added the kernprof CLI flags for controlling both:
  • --prof-star-imports:
    Whether to handle star-imports (old behavior is false)
  • --prof-nested-imports=...:
    What kind(s) of code blocks ProfmodExtractor should descend into to scan for imports
• Added the corresponding line_profiler.toml config options:
  • [tool.line_profiler.autoprofile]::prof_star_imports and ::import_discovery
  • [tool.line_profiler.kernprof]::prof-star-imports and ::prof-nested-imports

The behavior of --prof-imports has also become more consistent with that of --prof-mod since now both are reined in by --prof-nested-imports=.... But while this is IMO a positive change, it is still a semantic change nonetheless.

See TTsangSC#6 for details.

Why this is still in draft

There's quite a lot of code in here, but as I was writing it it felt increasingly apparent to me that:

• I'm writing around the current code, and doing quite a bit of busy work in the name of backward compatibility, and
• Having both ProfmodExtractor and AstProfileTransformer crawl and transform (in the case of ProfmodExtractor, indirectly via AstTreeProfiler._profile_ast_tree()) the AST only added (1) overhead and (2) unnecessary bookkeeping for consistency and prevention of duplication.

Cutting out the middleman (ProfmodExtractor) and letting AstProfileTransformer alone handle the profiling of imports would fix both, but it is probably not trivial to do it in a backward-compatible way. Hence I figured I'd just put the code out here first, so that we sit on it for a while, and weigh whether to incorporate the changes as-is, or to wait until the next release where we can be more haphazard with changing (esp. removing) and refactoring repo components.

See TTsangSC#7 for the proposed further changes.

[codecov]

Codecov Report

❌ Patch coverage is 92.99363% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.82%. Comparing base (b5ca752) to head (94df85a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...ne_profiler/autoprofile/ast_profile_transformer.py	87.94%	14 Missing and 3 partials ⚠️
line_profiler/autoprofile/profmod_extractor.py	97.22%	1 Missing and 2 partials ⚠️
line_profiler/autoprofile/line_profiler_utils.py	92.30%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #440      +/-   ##
==========================================
+ Coverage   84.95%   85.82%   +0.87%     
==========================================
  Files          21       21              
  Lines        2412     2667     +255     
  Branches      376      419      +43     
==========================================
+ Hits         2049     2289     +240     
- Misses        263      276      +13     
- Partials      100      102       +2     

Files with missing lines	Coverage Δ
line_profiler/autoprofile/_import_targets.py	91.66% <100.00%> (+1.52%)	⬆️
line_profiler/autoprofile/ast_tree_profiler.py	100.00% <100.00%> (+3.33%)	⬆️
line_profiler/autoprofile/autoprofile.py	91.42% <100.00%> (+0.51%)	⬆️
line_profiler/autoprofile/run_module.py	77.08% <100.00%> (+0.48%)	⬆️
line_profiler/autoprofile/line_profiler_utils.py	76.74% <92.30%> (+23.80%)	⬆️
line_profiler/autoprofile/profmod_extractor.py	90.09% <97.22%> (+6.13%)	⬆️
...ne_profiler/autoprofile/ast_profile_transformer.py	87.22% <87.94%> (+2.84%)	⬆️

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ef5262...94df85a. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Erotemic]

I ran a GPT 5.6 review on this:

Details

Yes. I would not merge #440 as-is, but I think the underlying feature work and much of the test suite are worth keeping.

The PR is still a draft, is currently mergeable, has 12 commits and 11 changed files, and is large: +2,361/−246. There are no submitted human reviews yet. CI is green across the build/test/typecheck matrix. Codecov reports about 93% patch coverage, with 22 changed lines uncovered, concentrated mainly in the new transformer.

The motivation is solid. #438 identifies real defects: selective --prof-mod misses nested imports, star imports aren't actually profiled, and the old global import-deduplication can cause an import inside one function to suppress the necessary hook in another function. The contributor also did a substantial job testing every major compound-statement category and adding E2E CLI tests.

Findings

1. Blocker: the two-pass AST design makes the new context-aware deduplication unstable. AstTreeProfiler._profile_ast_tree() stores already-profiled imports keyed by paths such as ('body', 1, 'body', 0), then inserts new statements into the AST, and only afterward runs AstProfileTransformer with those old paths. An insertion into an ancestor statement list changes the indices of descendants without updating their recorded paths.

   A small example is:

   import os
   
   if condition:
       from textwrap import indent

   Suppose both imports are selected, the script itself is also in --prof-mod, --prof-imports is enabled, and conditionals are enabled—which is the PR's default. The nested import is initially under body[1]. The first pass inserts a hook after import os, moving the if to body[2]. The dedup map still says the nested import is under body[1]. When AstProfileTransformer gets there, it does not recognize the earlier insertion and adds another hook.

   The PR's deduplication test specifically verifies imports in multiple function scopes, but it has no earlier ancestor-level import that mutates the indices of those functions, so it does not expose this case.

   This probably doesn't corrupt profiling results because adding a callable is intended to be idempotent, but these hooks can recursively inspect large classes/modules; the PR itself calls out that their cost can be substantial. More importantly, this is exactly the class of bookkeeping problem the contributor worried the two-pass architecture would create.

   I would fix this structurally rather than repairing indexes. A single transformer should decide, when it reaches each import, whether the import matches prof_mod, whether profile_imports overrides target filtering, whether its enclosing constructs permit discovery, and whether star imports are enabled. If two passes absolutely have to remain, deduplicate using the identity of the original ast.Import/ast.ImportFrom node rather than its mutable positional path.

2. Blocker for the star-import feature: add_star_import() does not reproduce Python's star-import semantics. It reopens the module with import_module(), enumerates it using inspect.getmembers(), filters that enumeration against __all__, and finally requires the re-read value to be identical to the object already in the destination namespace.

   That's problematic for PEP 562-style modules. For example:

   __all__ = ['dynamic']
   
   def __getattr__(name):
       if name == 'dynamic':
           return make_something()

   from mod import * can import dynamic because it explicitly retrieves names from __all__; inspect.getmembers(mod) need not find dynamic at all if it is absent from dir(mod). And if __getattr__ constructs a fresh object on each access, the is check can reject the successfully imported object anyway.

   There is also unnecessary behavioral risk: inspect.getmembers() can access names that the real star import did not access, particularly with a custom __dir__.

   The destination namespace is already the ground truth. I would implement this as roughly: use module.__all__ when present, otherwise use public keys from vars(module); then profile namespace[name] for those names actually present. Don't re-fetch each object's value from the module.

3. I agree with the contributor's architectural concern, and findings 1–2 make it more than cleanup. The contributor explicitly notes that the implementation now has two or three AST passes and proposes folding import selection directly into AstProfileTransformer, eliminating ProfmodExtractor as the intermediary. I think that is the right direction.

   There is now a lot of machinery whose purpose is keeping the two traversals synchronized: _ImportFinder, mutable AST location paths, _ContextAwareDuplicateChecker, updating stored ImportTarget.index values as expressions are inserted, separate discovery-selection maps, and then a second traversal trying to respect all of that. The ancestor-index bug is a direct consequence.

There is also one policy decision I would settle during that refactor: should existing --prof-imports behavior change by default? Historically, whole-script --prof-imports descended arbitrarily deeply. This PR makes it obey --prof-nested-imports and defaults function bodies and loops off. The contributor explicitly identifies that as a semantic change. I would lean toward preserving existing --prof-imports semantics by default and making restricted discovery opt-in, unless we deliberately put this in a release where that compatibility change is acceptable. Selective --prof-mod discovery can still gain sensible nested-import behavior without silently making --prof-imports less comprehensive.

What I would do with the PR

I would ask TTsangSC to keep the feature specification and most of the tests, but simplify the implementation around one authoritative AST transformation pass. The tests are actually one of the strongest parts of the contribution; they encode nested discovery, star-import filtering, CLI behavior, and scope-aware deduplication quite comprehensively.

So my maintainer verdict would be request changes, architectural rather than incremental. I would not ask them to throw the work away. The core requirements are good, and their own PR description already points toward essentially the same refactor. After that refactor, I would add two regression tests specifically for the ancestor-index-shift case and dynamic __all__/__getattr__ star exports.

I think its not worth maintaining backwards compat, and we should target this refactor for a line-profiler 6.0 feature, as such I also ran a GPT 5.6 review against your branch. I've read the main bullets, some details, and generally agree with the assessment:

Details

Yes. PR #7 is the much better basis if we are willing to call this line_profiler 6.0 work. The author has independently arrived at essentially the same architectural objection we found in #440: multiple AST passes are forcing fragile bookkeeping, so #7 introduces a single authoritative transformer.

I would not merge #7 exactly as written, though. I would use it as the prototype for a cleaner 6.0 rewrite and explicitly tell the contributor that we do not need the large backward-compatibility shell.

Review findings

1. The central architectural change is correct. SinglePassTransformer handles import selection/instrumentation, function decoration, and relative-import handling during one traversal. This removes the stale-AST-location problem I identified in ENH: handle nested and star imports #440: there is no longer a first pass mutating statement lists followed by another pass trying to recognize old positional paths.

   So I would keep this direction.

2. For 6.0, delete the old/new core mechanism entirely. Dev/tweaks #7 currently keeps the old AstTreeProfiler, AstProfileTransformer, ProfmodExtractor, and run_module interfaces, adds LINE_PROFILER_AUTOPROFILE_CORE, and parametrizes tests against "old" and "new".

   This is exactly the compatibility burden we don't need. The author says removing the superseded interfaces cuts roughly 1.1k lines. I think we should take that reduction.

   For 6.0 I would remove the environment switch, _rewrite_ast_legacy, the old/new test matrix, and probably the four superseded modules outright unless there is a small genuinely useful API worth retaining under a new name. A major version is the right time.

3. There is still a real correctness bug in star-import handling inherited from ENH: handle nested and star imports #440. add_star_import() reconstructs a star import using inspect.getmembers(module), then compares those values by identity with the already-imported namespace.

   That does not implement Python's actual from m import * semantics. For example:

   # m.py
   __all__ = ["dynamic"]
   
   def __getattr__(name):
       if name == "dynamic":
           return make_object()
       raise AttributeError(name)

   from m import * successfully imports dynamic, but inspect.getmembers(m) need not contain it at all. I verified that case. The helper will therefore silently fail to profile an object Python successfully imported.

   The solution is simpler than the current code: the destination namespace is authoritative. If __all__ exists, iterate those names and retrieve their values from namespace; otherwise use the public keys in vars(module) and retrieve those values from namespace. Don't rediscover the values through inspect.getmembers() and don't require identity with a second attribute lookup.

4. The new core is still carrying too much compile-time dedup machinery. _ConcreteDuplicateImportChecker records imports by AST context and suppresses any later import with the same ImportTarget.name in that context.

   That has a dynamic-Python correctness hole:

   from dep import func
   dep.func = replacement
   from dep import func
   func()

   Both import statements have the same source target dep.func. The transformer instruments the first and statically suppresses the second, even though the second binding can refer to a different callable.

   More importantly for architecture, this static deduplication is why Dev/tweaks #7 still needs ContextAwareVisitor, AST location tracking, insertion offsets, ImportTarget.index, _DuplicateImportChecker, etc. ImportTarget still carries an AST-body index largely for this machinery.

   For 6.0 I would move deduplication toward runtime/object-aware idempotence instead of source-position deduplication. Then the transformer can simply instrument every selected import it encounters. A cheap fast path in the profiling hook can avoid repeatedly traversing an already-seen callable/class/module. This is both semantically closer to Python and architecturally much smaller.

5. Don't rewrite relative imports merely to understand them. The single-pass transformer currently converts from .foo import bar to its absolute equivalent in the emitted AST before passing it onward. That's inherited from the old ImportFromTransformer, but we no longer need to preserve that architecture.

   Resolve the absolute module name internally for matching --prof-mod and for the profiling hook, but leave the user's ImportFrom node alone. Then run_module.py and ImportFromTransformer can disappear, and our AST transformation has less behavioral surface.

6. Configuration should terminate before the transformer. Dev/tweaks #7 adds autoprofile.prof_func_defs and autoprofile.prof_explicit_imports configuration values, but the actual autoprofile.run() path computes those values itself from prof_mod / profile_imports and explicitly passes them into SinglePassTransformer. The config defaults therefore mostly serve direct construction of a class in a private _single_pass_transformer module. That's unnecessary surface.

   I would have kernprof/autoprofile.run() resolve configuration and CLI inputs once into a small explicit plan/options object. The AST transformer should consume resolved policy, not know how to search or reload TOML files. That also lets us delete _get_conf_table(), whose ability to climb out of a ConfigSource subtable by reopening its source exists primarily to support all the compatibility entry points.

There is an additional simplification worth considering once we stop preserving the old implementation: _ProfModHelper.to_dotted_paths() still recursively enumerates every submodule underneath package targets. For --prof-mod=numpy, we shouldn't necessarily need to walk NumPy's package tree just to determine that an AST import of numpy.linalg is under the requested prefix. Prefix-aware module matching could eliminate more filesystem/import-discovery work.

What I would make the 6.0 architecture

I think the target should be approximately:

kernprof CLI/config
        |
        v
resolve_autoprofile_plan(...)
        |
        v
SinglePassTransformer(plan)
        |
        +-- decorate selected local defs
        +-- recognize imports
        +-- resolve relative module names for matching only
        +-- insert runtime hook after selected imports
        |
        v
compile + execute
        |
        v
runtime import hook
        +-- cheap object-aware idempotence
        +-- explicit imports
        +-- star-import namespace handling

No ProfmodExtractor. No AstTreeProfiler. No AstProfileTransformer compatibility facade. No ImportFromTransformer. No old/new switch. Ideally no AST location bookkeeping.

The behavioral tests from #440/#7 are valuable and should survive. I would add three regressions before considering the 6.0 version finished: the dynamic __all__/__getattr__ star-import case; repeated identical source imports that bind different objects; and kernprof -m with relative imports while asserting we don't rewrite those imports unnecessarily.

So the verdict is: yes, pivot from #440 to #7, but give TTsangSC permission to go substantially farther than #7. The single-pass insight is right; the remaining problem is that #7 treats it as a backward-compatible alternate implementation. For 6.0, we should instead make it the sole implementation and use the opportunity to delete most of the machinery it was designed to supersede.

[TTsangSC]

Very solid points; and thanks especially for also taking a look at TTsangSC#7. Will get to that in the following days.

Would you prefer that I work on dissecting #431 first, or on streamling this one first, and thus also leaving that feature for 6.0?

Also interesting that you've switched to GPT for the recent reviews. Is this where the current state of the art is at? Do you find it more capable and/or cost-effective than the corresponding Anthropic offerings nowadays?

[Erotemic]

I think line-profile information for multiprocessing workflows has a much higher utility than import stars (surely real developers would never use such cursed expressions), whereas multiprocessing is extremely common, and I think it would be more beneficial for users.

Also interesting that you've switched to GPT for the recent reviews.

I have subscriptions for both. I've found the 5.6 model to be a big step over previous 5.4 and 5.5 iterations, but that is anecdotal. I'm considering looking at some of the other models like Kimi K3, Qwen3.8 Max, GLM 5.3, or DeepSeek 4.1 flash. (ad apparently muse spark 1.3 is a thing now?) And I really need to give local models another look.

[TTsangSC]

Fair point; on it.