Skip to content

Conversation

janfb
Copy link
Contributor

@janfb janfb commented Aug 13, 2025

In PR #1633, I noticed that the typing module defines NewType instead of using TypeAlias. For our purposes, an alias is just fine and comes without consequences for documentation rendering and type checking (NewType does not implicitly perform type checking according to the actual underlying type). Therefore:

  • Replace NewType with TypeAlias for SummaryWriter, Distribution, Tensor, Transform.
  • Rename TensorboardSummaryWriter → TensorBoardSummaryWriter.
  • Remove unused TorchModule alias; use torch.nn.Module where needed (e.g., NPE_A.build_posterior).
  • Update imports/annotations across trainers to new alias.

Notes:

  • Type-only refactor; no behavior changes.
  • Breaking rename: external imports of TensorboardSummaryWriter/TorchModule must update.

@janfb janfb requested a review from manuelgloeckler August 13, 2025 14:48
Copy link

codecov bot commented Aug 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@ebb8c6f). Learn more about missing BASE report.
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1637   +/-   ##
=======================================
  Coverage        ?   82.55%           
=======================================
  Files           ?      135           
  Lines           ?    10948           
  Branches        ?        0           
=======================================
  Hits            ?     9038           
  Misses          ?     1910           
  Partials        ?        0           
Flag Coverage Δ
unittests 82.55% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
sbi/inference/trainers/nle/mnle.py 95.45% <100.00%> (ø)
sbi/inference/trainers/nle/nle_a.py 100.00% <100.00%> (ø)
sbi/inference/trainers/npe/mnpe.py 95.45% <100.00%> (ø)
sbi/inference/trainers/npe/npe_a.py 63.91% <100.00%> (ø)
sbi/inference/trainers/npe/npe_b.py 100.00% <100.00%> (ø)
sbi/inference/trainers/npe/npe_c.py 73.33% <100.00%> (ø)
sbi/inference/trainers/nre/bnre.py 44.82% <100.00%> (ø)
sbi/inference/trainers/nre/nre_a.py 100.00% <100.00%> (ø)
sbi/inference/trainers/nre/nre_b.py 100.00% <100.00%> (ø)
sbi/inference/trainers/nre/nre_c.py 100.00% <100.00%> (ø)
... and 1 more

Copy link
Contributor

@manuelgloeckler manuelgloeckler left a comment

Choose a reason for hiding this comment

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

Looks good.

Regarding typing for PyTorch > 2.6 (by now we are already at 2.8). So I dont think this will get fixed in a while (and it might be best to ignore the errors and allow the newer versions). I think the underlying reason is that the updated new version does not come with updated/correct type "stubs" (I some extra files which does give type hints for external non-python function to pyright) . Its just that certain "inplace" operations on Tensors are not recognized anymore by pyright.

@janfb janfb merged commit 67d7e1c into main Aug 14, 2025
9 checks passed
@janfb janfb deleted the fix-types-aliases branch August 14, 2025 10:20
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.

2 participants