Detect local arguments and functions used in a function by samwaseda · Pull Request #164 · pyiron/flowrep

samwaseda · 2026-02-26T08:36:43Z

Following the effort in #155, I started trying to parse variables as well. Edits will follow....

github-actions · 2026-02-26T08:36:54Z

👈 Launch a binder notebook on branch pyiron/flowrep/args

samwaseda · 2026-02-26T08:42:22Z

@liamhuber I started looking into the possibility of including variables as well. Frankly I'm also not really convinced but I still would like to explore the possibility

codecov · 2026-02-26T08:43:22Z

Codecov Report

❌ Patch coverage is 89.23077% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.42%. Comparing base (a7e3b65) to head (6581e93).

Files with missing lines	Patch %	Lines
src/flowrep/parsers/dependency_parser.py	89.23%	7 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##              main     #164      +/-   ##
===========================================
- Coverage   100.00%   98.42%   -1.58%     
===========================================
  Files           30       30              
  Lines         2009     2035      +26     
===========================================
- Hits          2009     2003       -6     
- Misses           0       32      +32

☔ 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.

samwaseda · 2026-02-26T13:54:40Z

I just had a discussion with @XzzX and we think we can keep the current implementation is maybe not quite as useless as I had imagined, because with this implementation it is now possible to throw an error as soon as an external variable is used.

liamhuber · 2026-02-26T19:19:41Z

because with this implementation it is now possible to throw an error as soon as an external variable is used.

Failing cleanly is infinitely better than looking like everything is well-provenanced when it isn't. However, I still think that restricting "ok locally defined stuff" to mean "don't use any symbols outside your function definition" is going to turn out to be a more annoying, more restrictive, more-work solution than telling people "you'll have to use well-versioned code to contribute to the KG". As an intermediate step towards catching and leveraging these external symbols, this still seems like solid progress though.

Copilot

Pull request overview

This PR updates dependency_parser to infer function dependencies by identifying undefined names in a function’s AST (including from type hints) and resolving those names from the function’s scope, recursively exploring unversioned objects.

Changes:

Replaced call-graph-based dependency crawling with undefined-variable detection and resolution.
Added UndefinedVariableVisitor and find_undefined_variables helpers to drive dependency discovery.
Reworked test_dependency_parser to focus on the new undefined-variable behavior and mocked resolution.

Reviewed changes

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

File	Description
`flowrep/models/parsers/dependency_parser.py`	Introduces undefined-name visitor + new dependency collection approach based on scope resolution and version availability.
`tests/unit/models/parsers/test_dependency_parser.py`	Replaces prior call-dependency tests with new tests for splitting, undefined-name detection, and mocked dependency discovery.

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

flowrep/models/parsers/dependency_parser.py

src/flowrep/parsers/dependency_parser.py

flowrep/models/parsers/dependency_parser.py

src/flowrep/parsers/dependency_parser.py

tests/unit/models/parsers/test_dependency_parser.py

flowrep/models/parsers/dependency_parser.py

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR modifies the dependency parsing logic to detect names used-but-not-defined in a function’s source (e.g., globals/type-hint symbols) and resolve them to objects for version tracking, as part of the ongoing dependency-crawling work from #155.

Changes:

Added an AST visitor (UndefinedVariableVisitor) plus find_undefined_variables to extract undefined symbols from source code.
Reworked get_call_dependencies to resolve those symbols via object_scope and optionally recurse for unversioned objects.
Replaced the prior call-graph oriented unit tests with new tests focused on undefined-variable discovery and resolution.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`flowrep/models/parsers/dependency_parser.py`	Adds undefined-variable detection and updates dependency collection logic to resolve those symbols to objects.
`tests/unit/models/parsers/test_dependency_parser.py`	Rewrites tests to cover the new undefined-variable based behavior and mocked resolution paths.

Comments suppressed due to low confidence (1)

tests/unit/models/parsers/test_dependency_parser.py:102

test_type_hints references np but the module isn't imported/defined anywhere in this test module, so defining test_function will raise NameError (or later get_call_dependencies will fail resolving it). Also, CI envs here don't include optional deps like NumPy, so importing NumPy in the test would be brittle. Define a small module-level dummy np (with an .array attribute) or use a stdlib object instead so the test is self-contained and doesn't add a new dependency.


if __name__ == "__main__":
    unittest.main()

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

src/flowrep/parsers/dependency_parser.py

flowrep/models/parsers/dependency_parser.py

+    def visit_Name(self, node):
+        if isinstance(node.ctx, ast.Load):  # Variable is being used
+            self.used_vars.add(node.id)
+        elif isinstance(node.ctx, ast.Store):  # Variable is being defined
+            self.defined_vars.add(node.id)
+
+    def visit_FunctionDef(self, node):
+        # Add the function name itself to defined variables
+        self.defined_vars.add(node.name)
+        # Add function arguments to defined variables
+        for arg in node.args.args:
+            self.defined_vars.add(arg.arg)
+        self.generic_visit(node)
+
+    def visit_AsyncFunctionDef(self, node):
+        # Add the async function name itself to defined variables
+        self.defined_vars.add(node.name)
+        # Add async function arguments to defined variables
+        for arg in node.args.args:
+            self.defined_vars.add(arg.arg)


src/flowrep/parsers/dependency_parser.py

samwaseda · 2026-03-19T18:06:09Z

@copilot Rewrite UndefinedVariableVisitor to forbid local function definitions.

Copilot · 2026-03-19T18:06:17Z

@samwaseda I've opened a new pull request, #190, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: samwaseda <37879103+samwaseda@users.noreply.github.com>

Detect local arguments and functions used in a function

Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>

Merge main into args

liamhuber · 2026-03-26T18:00:21Z

Ok super, the commit history looks reasonable here 😅 merge conflict resolution worked 🚀

samwaseda · 2026-03-27T06:22:36Z

Ah the imports from #158 disappeared XD

liamhuber · 2026-03-27T13:44:55Z

I wonder if ruff reformatted them to the top of the file. I'll look into it later this morning.

Is finding them inside the function parsing not an element of the tests?

liamhuber · 2026-03-27T14:23:40Z

@samwaseda, ah, shoot. Main never got merged into this PR since #158 got merged into main. Since this PR only touched dependency_parser and its test, I thought this branch had the cutting-edge of dependency_parser. I'll reset the head to before I messed around with it and force-push, but the merge will need to be re-done at some point...

samwaseda · 2026-03-27T14:30:21Z

It’s ok leave it like this. I’ll take care of it

liamhuber · 2026-03-27T14:41:21Z

No, I broke it. I think I can (mostly) fix it.

@samwaseda

In #164. I'm leveraging the diff comparison between main and the last commit in that PR that I didn't touch: https://github.com/pyiron/flowrep/compare/1e893dede850a43f2e0d3cc5595f3cb62fda1da9..e334f1f8815fc52e70d472e485dab9a024216437. Weirdly, most of the files identified as "moved" from flowrep/models to src/flowrep, but the two relevant files (`(test_)dependency_parser.py`) showed as completely removed and completely re-written. This caused me to completely delete @samwaseda's actual work in that branch during the merge. Here I lose the git history, but recover the actual file changes there. Plus I ran black and updated the module paths to reflect the current location of file in main. Actual full author: Co-authored-by: Sam Dareska <37879103+samwaseda@users.noreply.github.com> Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>

@samwaseda

In #164. I'm leveraging the diff comparison between main and the last commit in that PR that I didn't touch: https://github.com/pyiron/flowrep/compare/1e893dede850a43f2e0d3cc5595f3cb62fda1da9..e334f1f8815fc52e70d472e485dab9a024216437. Weirdly, most of the files identified as "moved" from flowrep/models to src/flowrep, but the two relevant files (`(test_)dependency_parser.py`) showed as completely removed and completely re-written. This caused me to completely delete @samwaseda's actual work in that branch during the merge. Here I lose the git history, but recover the actual file changes there. Plus I ran black and updated the module paths to reflect the current location of file in main. (actually same is primary author) Co-authored-by: Sam Dareska <37879103+samwaseda@users.noreply.github.com> Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>

liamhuber · 2026-03-27T14:53:00Z

K, I couldn't figure out a fix to recover the full git history, but I think I got the content recovered and consistent with main in #214

samwaseda added 7 commits February 25, 2026 18:32

Current state

cede0a1

Merge branch 'resolve' into args

9386d7e

Refactor items

87604aa

Rename some stuff

995b9ef

Move get_dependencies

f0b255c

safe guard for inline access to members

bb5d7a5

Add tests and black

0862a51

samwaseda changed the base branch from main to crawler February 26, 2026 08:36

samwaseda marked this pull request as draft February 26, 2026 08:37

samwaseda added 2 commits February 26, 2026 09:38

future annotation

c6c0fa1

mypy

c1c7978

samwaseda marked this pull request as ready for review February 26, 2026 13:55

This was referenced Feb 26, 2026

Detect all local functions and imported functions recursively #155

Merged

[refactor] Separate transformation to object from ast parsing #161

Merged

Base automatically changed from crawler to main February 27, 2026 07:20

Merge remote-tracking branch 'origin' into args

a646a44

liamhuber mentioned this pull request Mar 5, 2026

Example of how atomic nodes are not policed #171

Closed

samwaseda and others added 8 commits March 8, 2026 18:39

Merge remote-tracking branch 'origin' into args

690c213

Create find_undefined_variables

740aa51

Merge remote-tracking branch 'origin' into args

ef0169a

Do not distinguish between functions and variables

ea31b27

go recursive only if there is no version

50503f6

black

9e6ee43

Add one more test

ee1c5ce

[dependabot skip] Update env file

e2b19d5

Merge remote-tracking branch 'origin' into args

09df6fd

samwaseda requested a review from Copilot March 19, 2026 16:01

Copilot AI reviewed Mar 19, 2026

View reviewed changes

samwaseda and others added 4 commits March 19, 2026 17:12

Potential fix for pull request finding

777a558

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

56aa4f2

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

3de03c5

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

dcd510b

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

samwaseda requested a review from Copilot March 19, 2026 17:53

Copilot started reviewing on behalf of samwaseda March 19, 2026 17:53 View session

Remove tests for now

2d48634

Copilot AI reviewed Mar 19, 2026

View reviewed changes

Initial plan

bc0accd

Copilot AI mentioned this pull request Mar 19, 2026

Detect local arguments and functions used in a function #190

Merged

Copilot AI and others added 5 commits March 19, 2026 18:10

Rewrite UndefinedVariableVisitor to forbid local function definitions

dbb1188

Co-authored-by: samwaseda <37879103+samwaseda@users.noreply.github.com>

Merge pull request #190 from pyiron/copilot/sub-pr-164

1e893de

Detect local arguments and functions used in a function

Merge main

87dbcf7

Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>

Fix module path in patch calls

d62ecc8

Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>

Merge pull request #211 from pyiron/merge-main-args

6581e93

Merge main into args

liamhuber mentioned this pull request Mar 27, 2026

(Repaired) Detect local arguments functions used in a function #214

Closed

Conversation

samwaseda commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 26, 2026

Uh oh!

samwaseda commented Feb 26, 2026

Uh oh!

codecov bot commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

samwaseda commented Feb 26, 2026

Uh oh!

liamhuber commented Feb 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

samwaseda commented Mar 19, 2026

Uh oh!

Copilot AI commented Mar 19, 2026

Uh oh!

liamhuber commented Mar 26, 2026

Uh oh!

samwaseda commented Mar 27, 2026

Uh oh!

liamhuber commented Mar 27, 2026

Uh oh!

liamhuber commented Mar 27, 2026

Uh oh!

samwaseda commented Mar 27, 2026

Uh oh!

liamhuber commented Mar 27, 2026

Uh oh!

liamhuber commented Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

samwaseda commented Feb 26, 2026 •

edited

Loading

codecov bot commented Feb 26, 2026 •

edited

Loading