[torchgen] Refactor `torchgen.utils.FileManager` to accept `pathlib.Path` by XuehaiPan · Pull Request #150726 · pytorch/pytorch

XuehaiPan · 2025-04-05T07:49:11Z

This PR allows FileManager to accept pathlib.Path as arguments while keeping the original str path support.

This allows us to simplify the code such as:

os.path.join(..., ...) with Path.__floordiv__(..., ...).

pytorch/torchgen/utils.py

Line 155 in 95a5958

template_path = os.path.join(self.template_dir, template_fn)

pytorch/torchgen/utils.py

Line 176 in 95a5958

filename = f"{self.install_dir}/{filename}"

os.path.basename(...) with Path(...).name.

pytorch/torchgen/utils.py

Line 161 in 95a5958

comment += f" from {os.path.basename(template_path)}"
Manual file extension split with Path(...).with_stem(new_stem)

pytorch/torchgen/utils.py

Lines 241 to 256 in 95a5958

    
           dot_pos = filename.rfind(".") 
        
           if dot_pos == -1: 
        
               dot_pos = len(filename) 
        
           base_filename = filename[:dot_pos] 
        
           extension = filename[dot_pos:] 
        
           for shard in all_shards: 
        
               shard_id = shard["shard_id"] 
        
               self.write_with_template( 
        
                   f"{base_filename}{shard_id}{extension}", filename, lambda: shard 
        
               ) 
        
           # filenames is used to track compiled files, but FooEverything.cpp isn't meant to be compiled 
        
           self.filenames.discard( 
        
               f"{self.install_dir}/{base_filename}Everything{extension}" 
        
           )

Stack from ghstack (oldest at bottom):

[ghstack-poisoned]

pytorch-bot · 2025-04-05T07:49:15Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

Large queue time for macos-m2-15 instances

❌ 1 New Failure

As of commit 0a437b7 with merge base e2ce17c ():

NEW FAILURE - The following job has failed:

pull / linux-focal-py3.13-clang10 / test (dynamo_wrapped, 2, 3, linux.2xlarge) (gh)
'test/test_reductions.py::TestReductionsCPU::test_sum_all_cpu_float64'

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

Copilot

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

Comments suppressed due to low confidence (1)

torchgen/utils.py:180

[nitpick] The error message does not properly interpolate the file variable; consider using an f-string (e.g. f"duplicate file write {file}") to enhance debugging clarity.

assert file not in self.files, "duplicate file write {file}"

[ghstack-poisoned]

pytorchmergebot · 2025-04-07T01:36:39Z

Rebased gh/XuehaiPan/269/orig onto refs/remotes/origin/viable/strict because #150732 was rebased, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/150726)

…ath` ghstack-source-id: a576b3b Pull Request resolved: pytorch#150726

[ghstack-poisoned]

pytorchmergebot · 2025-04-08T01:54:25Z

Rebased gh/XuehaiPan/269/orig onto refs/remotes/origin/viable/strict because #150732 was rebased, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/150726)

pytorchmergebot · 2025-04-08T06:39:50Z

Starting merge as part of PR stack under #150732

[ghstack-poisoned]

…ath` ghstack-source-id: e910556 Pull Request resolved: pytorch#150726

[ghstack-poisoned]

…ath` ghstack-source-id: 4f12c6e Pull Request resolved: pytorch#150726

[ghstack-poisoned]

…ath` ghstack-source-id: b1198ff Pull Request resolved: pytorch#150726

aorenste

Looks pretty good w/ a few changes needed:

You should probably mention the change to assert_never in the summary.
For BC we probably need to keep assert_never and mark it w/ deprecated.

aorenste · 2025-05-14T14:56:17Z

torchgen/utils.py

-        self.filenames = set()
+    def __init__(
+        self,
+        install_dir: str | Path,


This syntax is python 3.10+ and we still need to support 3.9. Please change these (here and below) to Union[str, Path]

There is a future import from __future__ import annotations at the top of the file.

torchgen/utils.py

[ghstack-poisoned]

aorenste · 2025-05-14T15:36:52Z

torchgen/utils.py

        raise


+if TYPE_CHECKING:


I'm not sure if the BC checker is "TYPE_CHECKING" or not... but if this passes the BC checker then I'm okay with it.

typing_extensions.assert_never provides the exact same function.

XuehaiPan · 2025-05-14T16:02:11Z

@pytorchbot merge

pytorchmergebot · 2025-05-14T16:04:08Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

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 · 2025-05-14T17:28:35Z

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

pull / linux-focal-py3.13-clang10 / test (dynamo_wrapped, 2, 3, linux.2xlarge)

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

…ath` ghstack-source-id: 2ebb98e Pull Request resolved: pytorch#150726

XuehaiPan · 2025-05-15T02:44:31Z

@pytorchbot merge -i

pytorchmergebot · 2025-05-15T02:46:42Z

Merge started

Your change will be merged while ignoring the following 1 checks: pull / linux-focal-py3.13-clang10 / test (dynamo_wrapped, 2, 3, linux.2xlarge)

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

…s (PEP 585) and Union Type (PEP 604) (#150727) #129001 (comment) is the motivation for the whole stack of PRs. In `torch/__init__.py`, `torch._C.Type` shadows `from typing import Type`, and there is no type stub for `torch._C.Type` in `torch/_C/__init__.pyi`. So we need to use `from typing import Type as _Type`. After enabling [Generic TypeAlias (PEP 585)](https://peps.python.org/pep-0585) in the `.pyi` type stub files, we can use `type` instead of `typing.Type` or `from typing import Type as _Type`. ------ - [Generic TypeAlias (PEP 585)](https://peps.python.org/pep-0585): e.g. `typing.List[T] -> list[T]`, `typing.Dict[KT, VT] -> dict[KT, VT]`, `typing.Type[T] -> type[T]`. - [Union Type (PEP 604)](https://peps.python.org/pep-0604): e.g. `Union[X, Y] -> X | Y`, `Optional[X] -> X | None`, `Optional[Union[X, Y]] -> X | Y | None`. Note that in `.pyi` stub files, we do not need `from __future__ import annotations`. So this PR does not violate issue #117449: - #117449 ------ Pull Request resolved: #150727 Approved by: https://github.com/aorenste ghstack dependencies: #150726

…nd Union Type (PEP 604) (#150728) #129001 (comment) is the motivation for the whole stack of PRs. In `torch/__init__.py`, `torch._C.Type` shadows `from typing import Type`, and there is no type stub for `torch._C.Type` in `torch/_C/__init__.pyi`. So we need to use `from typing import Type as _Type`. After enabling [Generic TypeAlias (PEP 585)](https://peps.python.org/pep-0585) in the `.pyi` type stub files, we can use `type` instead of `typing.Type` or `from typing import Type as _Type`. ------ - [Generic TypeAlias (PEP 585)](https://peps.python.org/pep-0585): e.g. `typing.List[T] -> list[T]`, `typing.Dict[KT, VT] -> dict[KT, VT]`, `typing.Type[T] -> type[T]`. - [Union Type (PEP 604)](https://peps.python.org/pep-0604): e.g. `Union[X, Y] -> X | Y`, `Optional[X] -> X | None`, `Optional[Union[X, Y]] -> X | Y | None`. Note that in `.pyi` stub files, we do not need `from __future__ import annotations`. So this PR does not violate issue #117449: - #117449 ------ Pull Request resolved: #150728 Approved by: https://github.com/cyyever, https://github.com/aorenste ghstack dependencies: #150726, #150727

Update

88bac1e

[ghstack-poisoned]

XuehaiPan requested review from albanD and soulitzer as code owners April 5, 2025 07:49

XuehaiPan added better-engineering Relatively self-contained tasks for better engineering contributors topic: not user facing topic category suppress-bc-linter Suppresses the failures of API backward-compatibility linter (Lint/bc_linter) labels Apr 5, 2025

pytorchbot added the open source label Apr 5, 2025

XuehaiPan changed the title ~~Refactor torchgen.utils.FileManager to accept pathlib.Path~~ [torchgen] Refactor torchgen.utils.FileManager to accept pathlib.Path Apr 5, 2025

XuehaiPan requested a review from Copilot April 5, 2025 08:25

Copilot AI reviewed Apr 5, 2025

View reviewed changes

Update

1cf7bc0

[ghstack-poisoned]

XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request Apr 7, 2025

[torchgen] Refactor torchgen.utils.FileManager to accept `pathlib.P…

c75b9b1

…ath` ghstack-source-id: a576b3b Pull Request resolved: pytorch#150726

albanD removed their request for review April 7, 2025 19:22

Update

91ab313

[ghstack-poisoned]

XuehaiPan requested a review from Skylion007 April 9, 2025 14:27

Update

4e6c4ce

[ghstack-poisoned]

Update

dbaa928

[ghstack-poisoned]

XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request May 2, 2025

[torchgen] Refactor torchgen.utils.FileManager to accept `pathlib.P…

7425cf3

…ath` ghstack-source-id: e910556 Pull Request resolved: pytorch#150726

Update

2b3654a

[ghstack-poisoned]

XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request May 10, 2025

[torchgen] Refactor torchgen.utils.FileManager to accept `pathlib.P…

ea2fc59

…ath` ghstack-source-id: 4f12c6e Pull Request resolved: pytorch#150726

Update

71e0a33

[ghstack-poisoned]

XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request May 14, 2025

[torchgen] Refactor torchgen.utils.FileManager to accept `pathlib.P…

bb78b62

…ath` ghstack-source-id: b1198ff Pull Request resolved: pytorch#150726

aorenste requested changes May 14, 2025

View reviewed changes

Update

0a437b7

[ghstack-poisoned]

XuehaiPan requested a review from aorenste May 14, 2025 15:25

aorenste reviewed May 14, 2025

View reviewed changes

aorenste approved these changes May 14, 2025

View reviewed changes

pytorchmergebot added the merging label May 14, 2025

pytorchmergebot removed the merging label May 14, 2025

XuehaiPan added a commit to XuehaiPan/pytorch that referenced this pull request May 14, 2025

[torchgen] Refactor torchgen.utils.FileManager to accept `pathlib.P…

778e4f6

…ath` ghstack-source-id: 2ebb98e Pull Request resolved: pytorch#150726

pytorchmergebot added the merging label May 15, 2025

pytorchmergebot closed this in 014726d May 15, 2025

pytorchmergebot removed the merging label May 15, 2025

github-actions bot deleted the gh/XuehaiPan/263/head branch June 18, 2025 02:22

	dot_pos = filename.rfind(".")
	if dot_pos == -1:
	dot_pos = len(filename)
	base_filename = filename[:dot_pos]
	extension = filename[dot_pos:]

	for shard in all_shards:
	shard_id = shard["shard_id"]
	self.write_with_template(
	f"{base_filename}{shard_id}{extension}", filename, lambda: shard
	)

	# filenames is used to track compiled files, but FooEverything.cpp isn't meant to be compiled
	self.filenames.discard(
	f"{self.install_dir}/{base_filename}Everything{extension}"
	)

Conversation

XuehaiPan commented Apr 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Apr 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

❗ 1 Active SEVs

❌ 1 New Failure

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

pytorchmergebot commented Apr 7, 2025

Uh oh!

pytorchmergebot commented Apr 8, 2025

Uh oh!

pytorchmergebot commented Apr 8, 2025

Uh oh!

aorenste left a comment

Choose a reason for hiding this comment

Uh oh!

aorenste May 14, 2025

Choose a reason for hiding this comment

Uh oh!

XuehaiPan May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aorenste May 14, 2025

Choose a reason for hiding this comment

Uh oh!

XuehaiPan May 14, 2025

Choose a reason for hiding this comment

Uh oh!

XuehaiPan commented May 14, 2025

Uh oh!

pytorchmergebot commented May 14, 2025

Merge started

Uh oh!

pytorchmergebot commented May 14, 2025

Merge failed

Uh oh!

XuehaiPan commented May 15, 2025

Uh oh!

pytorchmergebot commented May 15, 2025

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

XuehaiPan commented Apr 5, 2025 •

edited

Loading

pytorch-bot bot commented Apr 5, 2025 •

edited

Loading

XuehaiPan May 14, 2025 •

edited

Loading