Skip to content

[Pyrefly][Refactor] Replace dict() calls with literal dict syntax for improved readability#157735

Closed
migeed-z wants to merge 1 commit intopytorch:mainfrom
migeed-z:fix_dict_literal
Closed

[Pyrefly][Refactor] Replace dict() calls with literal dict syntax for improved readability#157735
migeed-z wants to merge 1 commit intopytorch:mainfrom
migeed-z:fix_dict_literal

Conversation

@migeed-z
Copy link
Contributor

@migeed-z migeed-z commented Jul 7, 2025

There are 31 places that I spotted which construct literal dictionaries.

This PR refactors dictionary construction by replacingdict(...)calls with literal {...} syntax where applicable.

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

@pytorch-bot
Copy link

pytorch-bot bot commented Jul 7, 2025

🔗 Helpful Links

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

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

⏳ No Failures, 1 Pending

As of commit 7bc7b66 with merge base c2510fc (image):
💚 Looks good so far! There are no failures yet. 💚

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

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 7, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: migeed-z / name: Zeina Migeed (7bc7b66)

@migeed-z migeed-z force-pushed the fix_dict_literal branch from 4cbafa3 to 7bc7b66 Compare July 7, 2025 21:22
) -> str:
buffer_size = " * ".join(map(str, size_args))
return KernelTemplate._template_from_string(self.ALLOCATE_WEIGHT_BUFFER).render(
dict(
Copy link
Collaborator

@Skylion007 Skylion007 Jul 7, 2025

Choose a reason for hiding this comment

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

If you really care about this, remove the C4 rule exception in our ruff config that allow this syntax.

Copy link
Contributor Author

@migeed-z migeed-z Jul 7, 2025

Choose a reason for hiding this comment

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

@Skylion007 thanks!
we definitely plan to support this syntax and while using the literal syntax is better, it seems too strict to ban it completely? cc @ndmitchell @ezyang

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean, it's mostly the pain of getting the codebase in compliance in the first place. I don't have an objection to the rule, especially since ruff can autofix it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, add noqas to test/ using ruff --add-noqa to keep the inductor tests for the old syntax and seems fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm working on just editing them to the right syntax. Will see how it goes :)

@migeed-z
Copy link
Contributor Author

migeed-z commented Jul 8, 2025

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jul 8, 2025
@pytorchmergebot
Copy link
Collaborator

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

@migeed-z
Copy link
Contributor Author

migeed-z commented Jul 8, 2025

@pytorchbot label "topic: not user facing"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants