[refactor] Separate transformation to object from ast parsing

[samwaseda]

I wanted to do something like object_scope.resolve_attribute_to_object("math.cos", scope) and realized that in the current format I would have to convert it to AST first, which makes little sense, so I refactored it.

To be honest, I would find it easier to simply run ast.unparse(node) instead of recursively loading it ourselves in resolve_symbol_to_object. What do you think? @liamhuber

[github-actions]

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.94%. Comparing base (d376d6b) to head (e6d8305).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #161      +/-   ##
==========================================
- Coverage   98.94%   98.94%   -0.01%     
==========================================
  Files          27       27              
  Lines        1903     1898       -5     
==========================================
- Hits         1883     1878       -5     
  Misses         20       20              

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

[Copilot]

Pull request overview

This PR refactors the object scope resolution logic to separate the transformation from AST nodes to objects from string-based attribute resolution. The main goal is to enable direct resolution of dot-separated attribute strings (like "math.cos") without requiring conversion to AST first.

Changes:

• Introduces resolve_attribute_to_object function to resolve dot-separated attribute strings directly
• Refactors resolve_symbol_to_object to delegate to the new function, reducing code duplication
• Adds if __name__ == "__main__" block to test file, consistent with project conventions

Reviewed changes

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

File	Description
flowrep/models/parsers/object_scope.py	Adds new resolve_attribute_to_object function and refactors resolve_symbol_to_object to use it
tests/unit/models/parsers/test_object_scope.py	Adds standard test runner block for direct execution

---

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

[Copilot]

The error message uses 'attr' which only contains the last attribute from the loop iteration, not necessarily the one that failed. This creates a misleading error message. For example, if "math.cos.nonexistent" fails at "cos", the error will show "Could not find attribute 'nonexistent' of math.cos.nonexistent". Consider capturing which specific attribute lookup failed or restructuring the error to be more accurate about what part of the chain caused the issue.

Suggested change

	raise ValueError(f"Could not find attribute '{attr}' of {attribute}") from e
	raise ValueError(f"Could not resolve attribute chain '{attribute}'") from e

[Copilot]

The new 'resolve_attribute_to_object' function lacks test coverage. While existing tests for 'resolve_symbol_to_object' indirectly exercise this function, there are no direct tests verifying its behavior with string inputs like "math.cos". Consider adding tests for this new public function to ensure it works correctly when called directly, which is the purpose mentioned in the PR description.

Suggested change

	raise ValueError(f"Could not find attribute '{attr}' of {attribute}") from e
	raise ValueError(f"Could not resolve attribute chain '{attribute}'") from e

[liamhuber]

Yeah, refactor looks totally fine to me. I do agree with the copilot comment that the new function should be separately tested.

To be honest, I would find it easier to simply run ast.unparse(node) instead of recursively loading it ourselves in resolve_symbol_to_object. What do you think? @liamhuber

My understanding is that ast.unparse will only get us the string back, so we'll still need scope and resolve_string_to_object to get an real, live object back. Further, ast.unparse will work on anything and won't restrict us to Name and Attribute chains. In the long run that might be fine, but right now we have restrictions on what subset of python syntax we're willing to parse. So there might be a more compact and efficient way to do this that uses unparse and then verifies the string rather than going step by step with _chain and verifying as we go, but right now we're probably looking at "kind of nice refactor" rather than a "does it all for us".

[liamhuber]

@samwaseda, do you intend to merge this? It looks like #164 re-introduces a less-well-documented version of the same refactor. If you want to use this we should probably get it in sooner rather than later to minimize resolving merge conflicts...