Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change file naming rules: replace with _ #69

Closed
wants to merge 2 commits into from

Conversation

zincnode
Copy link
Contributor

@zincnode zincnode commented Nov 23, 2024

  • Update file naming rules
  • Add .gitignore rules for tests output
  • Update tests

For code review, you can just focus on: 05cc7bb

- Update file naming rules
- Add `.gitignore` rules for tests output
@zincnode
Copy link
Contributor Author

File names containing spaces are not friendly to Linux users, so I would like to replace the in the file name with _.

@youkaichao
Copy link
Member

Hi, thanks for your interest! torch.compile internally uses _ a lot, like __compiled_fn_1 , so I added #70 to use mixed dot and underscore. underscore is only used for separating strings, and dot is used to create the hierarchy.

the change looks like:

rename tests/depyf_output/multiprocessing/{__compiled_fn_1 Captured Graph 0.py => __compiled_fn_1.Captured_Graph.0.py}

Hope it works for your Linux machine.

@youkaichao
Copy link
Member

Add .gitignore rules for tests output

You can create another pr for this change 👍

@zincnode
Copy link
Contributor Author

Hi, thanks for your interest! torch.compile internally uses _ a lot, like __compiled_fn_1 , so I added #70 to use mixed dot and underscore. underscore is only used for separating strings, and dot is used to create the hierarchy.

the change looks like:

rename tests/depyf_output/multiprocessing/{__compiled_fn_1 Captured Graph 0.py => __compiled_fn_1.Captured_Graph.0.py}

Hope it works for your Linux machine.

Thanks for the quick response. Better idea.

@zincnode
Copy link
Contributor Author

Add .gitignore rules for tests output

You can create another pr for this change 👍

OK, I'll close this PR.

@zincnode zincnode closed this Nov 24, 2024
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