Conversation
|
I went ahead and merged main to drop the diff down to the two relevant files. Tests should work now too |
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #155 +/- ##
==========================================
+ Coverage 98.94% 98.97% +0.02%
==========================================
Files 27 28 +1
Lines 1901 1942 +41
==========================================
+ Hits 1881 1922 +41
Misses 20 20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
And return a map between the version info and the usages. Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
liamhuber
left a comment
There was a problem hiding this comment.
I'm not sure yet exactly how you plan to use it, but it's a nice direction. TIL about ast.NodeVisitor!
There's a couple places where we can swap out the helper functions with stuff from flowrep.models.parsers.object_scope that is all fully tested. Otherwise, my only big concern was the asymmetry of what you get back depending on whether the function is "local" or not.
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
|
(I cherry-picked one commit because git diff was difficult to read) |
|
@liamhuber I didn't understand why you were returning a list of (same?) functions in the returned dictionary. Was it because my intention of this module was not clear? |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new crawler module that provides functionality to recursively detect all callable dependencies of a Python function through AST introspection. The implementation tracks both local functions and imported functions, capturing version information where available to support reproducibility tracking.
Changes:
- Added
flowrep/crawler.pywith core dependency tracking functionality - Added comprehensive unit tests in
tests/unit/test_crawler.py - Implemented recursive dependency resolution with cycle detection
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 13 comments.
| File | Description |
|---|---|
| flowrep/crawler.py | Core implementation with get_call_dependencies() for recursive dependency tracking, split_by_version_availability() for partitioning dependencies by version info, and CallCollector AST visitor for extracting function calls |
| tests/unit/test_crawler.py | Comprehensive test suite covering basic behavior, transitive dependencies, diamond patterns, cycle detection, and version availability partitioning |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Oh, really excellent catch. Because I did it right before bed and I was being dumb. The relevant changes are I see you've switched it back to a single |
Yeah, you're 100% right, it's just Then some of the AI complaints about "dependent" vs "dependents" are correct and need to be acted on. |
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
# Conflicts: # flowrep/crawler.py
|
Copilots comments on the tests are largely reasonable, and the coverage is still lacking, so a bit more there would be nice |
This is only reachable if something is identified by ast as a `Call`, but _isn't_ callable. This should only happen in contrived situations. Let's just fail hard and ask for clarity. Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
|
I think I'm done here @samwaseda. I believe it does fairly what it says it will do -- |
liamhuber
left a comment
There was a problem hiding this comment.
@samwaseda, we talked about spinning some files off into a new repo at some point, but in the meantime I'm happy to merge these developments here. I would only try to avoid getting it too entangled with the other parsing. This is completely standalone and #164 is just making some QoL changes to the object_scope that are already in #161, so so far so good.
Following today's pyiron meeting, I wrote a function to detect all local functions and imported functions used in a functions recursively. This in principle ensures reproducibility, since all packages would be correctly tracked (including versions).
Example:
Output:
({<function add at 0x1034af690>, <function op at 0x103bdd220>}, {VersionInfo(module='math', qualname='sqrt', version='3.14.3')}).I have no idea how reliably this works. Anyway, despite
pyiron_snippetsv.1.1.0 not fully published, I wanted to open this PR so that @liamhuber can review it before I wake up tomorrow morning 😎.