Skip to content

use "SimState" type rather than Self in state.py#191

Closed
curtischong wants to merge 1 commit intomainfrom
add-types2
Closed

use "SimState" type rather than Self in state.py#191
curtischong wants to merge 1 commit intomainfrom
add-types2

Conversation

@curtischong
Copy link
Copy Markdown
Collaborator

@curtischong curtischong commented May 12, 2025

Summary

This fixes a few type errors I've been getting via pylance.

I've talked to Janosh about the larger typing issues in the codebase but I currently do not have time to look into those right now. So I want to merge this PR in so others don't think I'm fixing all of the other typing issues.

Checklist

Before a pull request can be merged, the following items must be checked:

  • Doc strings have been added in the Google docstring format.
    Run ruff on your code.
  • Tests have been added for any new functionality or bug fixes.

We highly recommended installing the pre-commit hooks running in CI locally to speedup the development process. Simply run pip install pre-commit && pre-commit install to install the hooks which will check your code before each commit.

@cla-bot cla-bot Bot added the cla-signed label May 12, 2025
@curtischong curtischong marked this pull request as draft May 12, 2025 16:10
@curtischong curtischong changed the title Add types2 use "SimState" type rather than Self in state.py May 14, 2025
cast list of simstates

more rename Self to SimState

linter fixes
@curtischong curtischong marked this pull request as ready for review May 14, 2025 17:23
Copy link
Copy Markdown
Member

@CompRhys CompRhys left a comment

Choose a reason for hiding this comment

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

LGTM, fairchem tests failures can be fixed by pinning the version in the test ci script (see: 1a48cb4)

Comment thread torch_sim/state.py
self.cell = value.transpose(-2, -1)

def clone(self) -> Self:
def clone(self) -> "SimState":
Copy link
Copy Markdown
Member

@CompRhys CompRhys May 14, 2025

Choose a reason for hiding this comment

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

I think typing as Self is correct per the pep? https://peps.python.org/pep-0673/

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

agree Self is better here (as in higher fidelity). it will correctly resolve to child classes of SimState like DeformState

https://github.com/Radical-AI/torch-sim/blob/8211e08d36b4ae73407564c78a1503f07f27b26b/tests/test_state.py#L489

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

interesting. pylance gave me errors for it though. maybe ty might not complain

@janosh janosh closed this May 14, 2025
@janosh janosh deleted the add-types2 branch June 10, 2025 13:50
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