Skip to content

Fix mypy errors: PyTreeSpec inheritance#160652

Closed
RohitRathore1 wants to merge 3 commits intopytorch:mainfrom
RohitRathore1:fix-cxx-pytree-mypy-errors
Closed

Fix mypy errors: PyTreeSpec inheritance#160652
RohitRathore1 wants to merge 3 commits intopytorch:mainfrom
RohitRathore1:fix-cxx-pytree-mypy-errors

Conversation

@RohitRathore1
Copy link
Collaborator

@RohitRathore1 RohitRathore1 commented Aug 14, 2025

Fixes #160650.

I added type ignore comment to LeafSpec class inheritance in torch/utils/_cxx_pytree.py to handle PyTreeSpec being marked as final in optree's type stubs.

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @chenyang78 @kadeng @muchulee8 @amjames @chauhang @aakhundov @coconutruben

@pytorch-bot
Copy link

pytorch-bot bot commented Aug 14, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/160652

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure

As of commit 7d12076 with merge base cd8d8c1 (image):

NEW FAILURE - The following job has failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@RohitRathore1
Copy link
Collaborator Author

@pytorchbot label "topic: not user facing"

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Aug 14, 2025
@XuehaiPan XuehaiPan requested a review from Skylion007 August 14, 2025 17:11
@mikaylagawarecki mikaylagawarecki added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Aug 14, 2025


class LeafSpec(TreeSpec, metaclass=LeafSpecMeta):
class LeafSpec(TreeSpec, metaclass=LeafSpecMeta): # type: ignore[misc,final]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the parent class final? Seems like a typing error upstream?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right. The PyTreeSpec class from the optree library is marked as @final in their type stubs, but PyTorch's LeafSpec has been inheriting from it successfully at runtime but this inheritance is working fine at runtime, the type ignore seems like the most pragmatic short-term fix while we could potentially discuss about the @final annotation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The LeafSpec class in _cxx_pytree.py is used to align the public APIs with _pytree.py. It is not intended to be used. Now, both Python and C++ pytree are using .is_leaf() method:

# previous code
isinsatnce(spec, LeafSpec)

# current
spec.is_leaf()

@XuehaiPan
Copy link
Collaborator

The CI failure is due to mypy only running on a single file.

@XuehaiPan
Copy link
Collaborator

@pytorchbot merge -f "MYPYSTRICT is only run on changed file. lint will be green on trunk"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

pytorchmergebot pushed a commit that referenced this pull request Aug 16, 2025
…unner-mypy` (#160806)

Like `MYPY`, linter `MYPYSTRICT` will need `--all-files` too.

See also:

- #160652 (comment)

Pull Request resolved: #160806
Approved by: https://github.com/seemethere
can-gaa-hou pushed a commit to can-gaa-hou/pytorch that referenced this pull request Aug 22, 2025
Fixes pytorch#160650.

I added type ignore comment to `LeafSpec` class inheritance in `torch/utils/_cxx_pytree.py` to handle `PyTreeSpec` being marked as final in optree's type stubs.

Pull Request resolved: pytorch#160652
Approved by: https://github.com/Skylion007
can-gaa-hou pushed a commit to can-gaa-hou/pytorch that referenced this pull request Aug 22, 2025
…unner-mypy` (pytorch#160806)

Like `MYPY`, linter `MYPYSTRICT` will need `--all-files` too.

See also:

- pytorch#160652 (comment)

Pull Request resolved: pytorch#160806
Approved by: https://github.com/seemethere
markc-614 pushed a commit to markc-614/pytorch that referenced this pull request Sep 17, 2025
Fixes pytorch#160650.

I added type ignore comment to `LeafSpec` class inheritance in `torch/utils/_cxx_pytree.py` to handle `PyTreeSpec` being marked as final in optree's type stubs.

Pull Request resolved: pytorch#160652
Approved by: https://github.com/Skylion007
markc-614 pushed a commit to markc-614/pytorch that referenced this pull request Sep 17, 2025
…unner-mypy` (pytorch#160806)

Like `MYPY`, linter `MYPYSTRICT` will need `--all-files` too.

See also:

- pytorch#160652 (comment)

Pull Request resolved: pytorch#160806
Approved by: https://github.com/seemethere
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged open source topic: not user facing topic category triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mypy type checking errors: PyTreeSpec inheritance

6 participants