Skip to content

feat: add globals if available - #319

Merged
samwaseda merged 1 commit into
mainfrom
safe_globals
Aug 25, 2026
Merged

feat: add globals if available#319
samwaseda merged 1 commit into
mainfrom
safe_globals

Conversation

@samwaseda

Copy link
Copy Markdown
Member

According to copilot, the problem I encounter here can be resolved with this change. Let's see.

@samwaseda
samwaseda requested review from liamhuber and a lite review from Copilot and removed request for liamhuber August 25, 2026 12:39
@github-actions

Copy link
Copy Markdown

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates scope resolution for parsing by merging a callable’s __globals__ (when present) into the resolved module scope, ensuring name lookups work even when inspect.getmodule() returns a module that isn’t actually the function’s defining global namespace.

Changes:

  • Extend get_scope to include func.__globals__ when available, before adding builtins.
  • Add a unit test asserting that function globals are preferred when the resolved module is unrelated.

Reviewed changes

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

File Description
tests/unit/parsers/test_object_scope.py Adds coverage for resolving names via a function’s __globals__ even when inspect.getmodule() indicates an unrelated module.
src/flowrep/parsers/object_scope.py Merges module globals with __globals__ (if present) before constructing the ScopeProxy and adding builtins.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@samwaseda samwaseda added patch backward compatible bug fixes priority: medium Extra attention is needed to figure out what is causing this error. labels Aug 25, 2026
@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.94%. Comparing base (4f65e6f) to head (119b746).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #319   +/-   ##
=======================================
  Coverage   99.94%   99.94%           
=======================================
  Files          44       44           
  Lines        3569     3586   +17     
=======================================
+ Hits         3567     3584   +17     
  Misses          2        2           

☔ View full report in Codecov by Harness.
📢 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
samwaseda merged commit 5a55ab6 into main Aug 25, 2026
30 checks passed
@samwaseda
samwaseda deleted the safe_globals branch August 25, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch backward compatible bug fixes priority: medium Extra attention is needed to figure out what is causing this error.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants