Skip to content

Fix resolving forward refs in generic aliases when ref type is not imported#871

Open
Borda wants to merge 8 commits intoomni-us:mainfrom
Borda:fix/708
Open

Fix resolving forward refs in generic aliases when ref type is not imported#871
Borda wants to merge 8 commits intoomni-us:mainfrom
Borda:fix/708

Conversation

@Borda
Copy link
Contributor

@Borda Borda commented Mar 17, 2026

What does this PR do?

This pull request addresses an issue with resolving forward references in generic type aliases, particularly when the referenced type is not imported into the module where the alias is used. The update ensures that such forward references are correctly resolved by tracing the origin module of the alias. Additionally, a regression test is added to verify this behavior.

Forward reference resolution improvements

  • Enhanced the logic in jsonargparse/_postponed_annotations.py to resolve string-based forward references in generic aliases (e.g., list["ForwardReferenced"]) by searching for missing types in the module where the alias was originally defined, not just the current module. This fixes cases where the forward-referenced type is not imported into the using module. [1] [2] [3]
  • Updated the changelog to document the fix for resolving forward references in generic aliases when the forward-referenced type is not imported in the using module.

Testing

  • Added a regression test (test_add_class_arguments_cross_module_forward_ref_708) to ensure that forward references in generic aliases are resolved correctly, even when the referenced type is not directly imported in the module where the alias is used.

Before submitting

  • Did you read the contributing guideline?
  • Did you update the documentation? (readme and public docstrings)
  • Did you write unit tests such that there is 100% coverage on related code? (required for bug fixes and new features)
  • Did you verify that new and existing tests pass locally?
  • Did you make sure that all changes preserve backward compatibility?
  • Did you update the CHANGELOG including a pull request link? (not for typos, docs, test updates, or minor internal changes/refactors)

closes #708
cc: @mauvilsa

Copilot AI review requested due to automatic review settings March 17, 2026 08:52
Copy link

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 fixes evaluation of postponed annotations when a generic alias contains a string forward reference (e.g. list["ForwardReferenced"]) and the referenced type isn’t imported into the module where the alias is used.

Changes:

  • Resolve str forward refs nested in generic alias __args__ during forward-ref resolution.
  • Enrich get_type_hints global namespace by locating missing forward-ref targets from the module that originally defined the imported generic alias.
  • Add a regression test for the cross-module forward-ref scenario and document the fix in the changelog.

Reviewed changes

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

File Description
jsonargparse/_postponed_annotations.py Adds global-namespace enrichment to resolve missing forward-ref targets for imported generic aliases.
jsonargparse_tests/test_postponed_annotations.py Adds regression test reproducing issue #708 via dynamically created/imported modules.
CHANGELOG.rst Documents the fix under the current release notes.

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

You can also share your feedback on Copilot code review. Take the survey.

@codecov
Copy link

codecov bot commented Mar 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (036294a) to head (a033d49).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #871   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        25           
  Lines         7398      7468   +70     
=========================================
+ Hits          7398      7468   +70     

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

Copy link

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

Improves evaluation of postponed annotations by resolving string-based forward references nested inside generic aliases even when the referenced type isn’t imported in the consuming module (regression fix for #708).

Changes:

  • Extend forward-ref resolution to handle str entries inside __args__ (e.g., list["ForwardReferenced"]).
  • Add global-scope enrichment logic to locate missing forward-referenced types from the module that defined the generic alias.
  • Add regression tests for cross-module generic-alias forward refs; document the fix in the changelog; update .gitignore.

Reviewed changes

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

File Description
jsonargparse/_postponed_annotations.py Adds logic to discover and inject missing forward-ref targets into get_type_hints globals and resolves string forward refs inside generic alias args.
jsonargparse_tests/test_postponed_annotations.py Adds regression tests covering cross-module import scenarios and non-module entries in sys.modules.
CHANGELOG.rst Notes the #708 fix under the next unreleased patch version.
.gitignore Adds IDE and local tooling artifacts to ignore list.

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

You can also share your feedback on Copilot code review. Take the survey.

Copy link

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 improves evaluation of postponed type annotations by resolving string forward references inside generic aliases (e.g., list["ForwardReferenced"]) even when the forward-referenced type is not imported in the consuming module, addressing issue #708.

Changes:

  • Extend resolve_forward_refs to resolve str entries found in __args__ (not only ForwardRef).
  • Add _enrich_globals_for_string_forward_refs with a trigger-to-module cache to locate missing forward-ref types from the alias origin module via sys.modules.
  • Add regression tests covering direct/indirect/aliased imports and cache behavior; update changelog and .gitignore.

Reviewed changes

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

File Description
jsonargparse/_postponed_annotations.py Adds logic to resolve string forward refs in generic aliases and attempts to enrich globals by tracing alias origin modules.
jsonargparse_tests/test_postponed_annotations.py Adds regression tests for #708 and targeted tests for globals enrichment and cache reuse/fallback behavior.
CHANGELOG.rst Documents the #708 fix under the next unreleased patch version.
.gitignore Adds IDE/tooling and local environment ignore entries.

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

You can also share your feedback on Copilot code review. Take the survey.

Borda and others added 4 commits March 19, 2026 08:58
… not imported (omni-us#708)

Two bugs in _postponed_annotations.py:
- resolve_subtypes_forward_refs only handled typing.ForwardRef args; on Python
  3.9+ PEP-585 generics (list["X"]) store forward ref args as plain str, so
  those were silently skipped. Now handles both ForwardRef and str uniformly.
- get_global_vars only collected globals from the using module, missing types
  that are referenced by string inside imported generic aliases but never
  directly imported. New _enrich_globals_for_string_forward_refs helper traces
  generic aliases back to their origin module via identity comparison and adds
  any missing referenced types to global_vars.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replaced value-based trigger tracking with identity-based tracking to support PEP-585 generics.
- Enhanced test coverage for aliased imports and partially imported references.
- Restructure tests into specialized classes using pytest fixtures.
- Add detailed cases for direct, indirect, and aliased imports.
- Improve coverage for _enrich_globals_for_string_forward_refs behavior.
Borda and others added 4 commits March 19, 2026 08:58
Co-authored-by: Codex <codex@openai.com>
…logic

- Introduced `_cache_trigger_module_name` helper for managing `_TRIGGER_MODULE_CACHE`.
- Added eviction of the oldest entries when exceeding `_TRIGGER_MODULE_CACHE_MAXSIZE`.
- Ensured deduplication of module names for the same trigger.
- Included tests for new caching behavior.
- Improved `_enrich_globals_for_string_forward_refs` with stricter type annotations.
- Added tests for nested generic aliases to ensure correct resolution of forward references.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- Added `_collect_string_fwd_ref_names` to process `ForwardRef` instances.
- Enhanced fallback for `get_global_vars` to handle source errors gracefully.
- Updated `_enrich_globals_for_string_forward_refs` to support stale cache entries and deduplicate triggers.
- Introduced several test cases covering edge scenarios in forward-reference resolution.
@Borda
Copy link
Contributor Author

Borda commented Mar 19, 2026

@mauvilsa updated with latest main, as rebase :)

@Borda Borda deployed to codecov March 19, 2026 08:00 — with GitHub Actions Active
Copy link
Member

@mauvilsa mauvilsa left a comment

Choose a reason for hiding this comment

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

Some weeks ago I tried an agent to fix #708, and even though it worked, I had some concerns. I am not sure if this fix has the same issues. I will analyze and get back to you.

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.

Weird behaviour in resolving class names when using forward references when the forward referenced class is not in scope

3 participants