Skip to content

Conversation

@MatthewFilipovich
Copy link
Owner

  • Update slicing behavior in System to match torch.nn.Sequential, returning a new System instance instead of a list of Elements.
  • Improve System docstring and dunder method type hints.

@MatthewFilipovich MatthewFilipovich requested a review from Copilot May 21, 2025 17:23
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 updates the System class to support slicing like torch.nn.Sequential, returning a new System instance rather than a list, and refines type hints and documentation.

  • Added @overload signatures and updated return types for __getitem__, plus type hints on __iter__ and __len__.
  • Changed slicing (system[i:j]) to return System instead of List[Element].
  • Enhanced docstring examples and expanded typing imports.

Reviewed Changes

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

File Description
torchoptics/system.py Introduced overloads, type hints, and updated docstring for slicing
tests/system/test_system.py Added basic slicing tests for System
Comments suppressed due to low confidence (1)

tests/system/test_system.py:166

  • Consider adding tests for slicing with steps (e.g., system[::2]) and negative indices (e.g., system[-2:]) to fully cover the new slicing behavior.
system_slice = system[:2]

@codecov
Copy link

codecov bot commented May 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (74ff76c) to head (5b0fe0c).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #63   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           37        37           
  Lines         1217      1223    +6     
=========================================
+ Hits          1217      1223    +6     

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

@MatthewFilipovich MatthewFilipovich merged commit 6fdae6b into main May 21, 2025
7 checks passed
@MatthewFilipovich MatthewFilipovich deleted the feature/system-like-sequential branch May 21, 2025 17:27
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