Skip to content

Quote nn.Module annotations so torch stays optional - #97

Merged
BSnelling merged 1 commit into
mainfrom
bes/restore-future-annotations
Nov 2, 2026
Merged

BSnelling merged 1 commit into
mainfrom
bes/restore-future-annotations

Conversation

@BSnelling

@BSnelling BSnelling commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

9b60487 removed from __future__ import annotations from nn_model.py, so the nn.Module annotations are evaluated at import time. Without torch installed, nn is undefined and importing petab_sciml (and therefore petab.v2) fails with NameError. Restore the future import.

This is in service of bumping the petab-sciml pin in AMICI, so we can avoid installing torch in more workflows: AMICI-dev/AMICI#3502

@BSnelling
BSnelling requested a review from dilpath November 2, 2026 11:34
@codecov-commenter

codecov-commenter commented Nov 2, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.16%. Comparing base (9d076fe) to head (3c11f48).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #97      +/-   ##
==========================================
+ Coverage   96.09%   96.16%   +0.07%     
==========================================
  Files          13       13              
  Lines         563      574      +11     
==========================================
+ Hits          541      552      +11     
  Misses         22       22              

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

@dilpath dilpath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we rather just reintroduce from __future__ import annotations -- what was the reason for removing that? Both fine for me

9b60487 removed `from __future__ import annotations` from nn_model.py,
so the nn.Module annotations are evaluated at import time. Without
torch installed, `nn` is undefined and importing petab_sciml (and
therefore petab.v2) fails with NameError.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@BSnelling
BSnelling force-pushed the bes/restore-future-annotations branch from 8eec916 to 3c11f48 Compare November 2, 2026 11:56
@BSnelling

Copy link
Copy Markdown
Collaborator Author

Given that you don't know the reason for removing from __future__ import annotations either, I'll update this PR to just restore it. That would be my preference in any case.

@BSnelling
BSnelling merged commit 76a2006 into main Nov 2, 2026
6 of 7 checks passed
@BSnelling
BSnelling deleted the bes/restore-future-annotations branch November 2, 2026 11:58
BSnelling added a commit to AMICI-dev/AMICI that referenced this pull request Nov 2, 2026
petab_sciml 9d076fe dropped `from __future__ import annotations` from
nn_model.py, so importing petab.v2 failed with NameError wherever torch
was not installed. Pin to 76a2006 (PEtab-dev/petab_sciml#97), which
restores it, and bring the docs requirements in line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

3 participants