Skip to content

Add import parser to deal with local imports#158

Open
samwaseda wants to merge 9 commits intomainfrom
import_parser
Open

Add import parser to deal with local imports#158
samwaseda wants to merge 9 commits intomainfrom
import_parser

Conversation

@samwaseda
Copy link
Member

Since local imports are not included in function.__globals__, I added an import parser to complete the scope dictionary, e.g.:

def cos(a):
    import math
    from numpy import linalg as la
    return math.cos(la.norm(a))

@samwaseda samwaseda changed the base branch from main to crawler February 24, 2026 10:09
@github-actions
Copy link

Binder 👈 Launch a binder notebook on branch pyiron/flowrep/import_parser

@samwaseda samwaseda requested a review from liamhuber February 24, 2026 10:09
@codecov
Copy link

codecov bot commented Feb 24, 2026

Codecov Report

❌ Patch coverage is 97.22222% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.94%. Comparing base (5491049) to head (e058027).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
flowrep/models/parsers/import_parser.py 95.45% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #158      +/-   ##
==========================================
- Coverage   98.97%   98.94%   -0.04%     
==========================================
  Files          28       29       +1     
  Lines        1949     1982      +33     
==========================================
+ Hits         1929     1961      +32     
- Misses         20       21       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

Copilot AI left a comment

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 adds an import parser to handle local imports in functions, which are not included in function.__globals__. The implementation parses import and from ... import ... statements from function ASTs and builds a scope dictionary that is merged with the module-level scope.

Changes:

  • Added import_parser.py module with build_scope() function to parse local imports from AST nodes
  • Modified CallCollector in crawler.py to collect import statements alongside function calls
  • Extended get_scope() in object_scope.py to accept optional extra modules from local imports
  • Added integration test to verify local imports are correctly detected in call dependencies

Reviewed changes

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

File Description
flowrep/models/parsers/import_parser.py New module that parses AST import nodes and dynamically imports modules to build a scope dictionary
flowrep/crawler.py Extended CallCollector to visit and collect import nodes, integrated import parser into dependency analysis
flowrep/models/parsers/object_scope.py Added optional extra_modules parameter to get_scope() for merging local imports
tests/unit/test_crawler.py Added test function with local imports and integration test validating they are captured

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

samwaseda and others added 4 commits February 24, 2026 14:45
@liamhuber
Copy link
Member

I see the necessity for crawler. This is also a good point that the ParseWorkflow should get extended to handle import syntax. However, I'm not comfortable modifying object_scope.get_scope in this way, since it lets you just fake existing scope.

Let me blatantly steal handle_Import ideas onto ParseWorkflow and modify the scope: ScopeProxy with it; for crawler let me think about it more -- on replying to the meeting thread I got scared that we're being too ambitious with the crawler anyhow 😬


def get_scope(func: FunctionType) -> ScopeProxy:
return ScopeProxy(inspect.getmodule(func).__dict__ | vars(builtins))
def get_scope(func: FunctionType, extra_modules: dict | None = None) -> ScopeProxy:
Copy link
Member

Choose a reason for hiding this comment

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

I'm not super comfortable with this. I'd rather leave get_scope alone, and then mutate a ScopeProxy object if and when we know there are new symbols available

Base automatically changed from crawler to main February 27, 2026 07:20
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.

3 participants