Skip to content

Update help message for 'path' argument to match user's environment #13937

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hunterhogan
Copy link
Contributor

Clarify the help message for the 'path' argument in the runtests.py script to reflect the correct format using PurePath. This change ensures that users receive accurate guidance on how to specify the path, improving usability on different operating systems.

Fixes #13936

args = parser.parse_args()
path: str = args.path
run_stubtest: bool = args.run_stubtest
python_version: str = args.python_version

path_tokens = Path(path).parts
if len(path_tokens) != 2:
parser.error("'path' argument should be in format <folder>/<stub>.")
parser.error(f"'path' argument should be in format {PurePath('<folder>', '<stub>')}.")
Copy link
Collaborator

@Avasam Avasam May 4, 2025

Choose a reason for hiding this comment

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

Isn't Path enough? (in Windows, it's a WindowsPath, which stringifies the way you want)

Suggested change
parser.error(f"'path' argument should be in format {PurePath('<folder>', '<stub>')}.")
parser.error(f"'path' argument should be in format {Path('<folder>', '<stub>')}.")
>>> from pathlib import Path
>>> path = Path('<folder>', '<stub>')
>>> print(path, type(path))
<folder>\<stub> <class 'pathlib._local.WindowsPath'>

I still would prefer fixing the support for forward-slashes. As that's intended to work.

@@ -58,15 +58,17 @@ def main() -> None:
choices=("3.9", "3.10", "3.11", "3.12", "3.13"),
help="Target Python version for the test (default: %(default)s).",
)
parser.add_argument("path", help="Path of the stub to test in format <folder>/<stub>, from the root of the project.")
parser.add_argument(
"path", help=f"Path of the stub to test in format {PurePath('<folder>', '<stub>')}, from the root of the project."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment/question as below

Suggested change
"path", help=f"Path of the stub to test in format {PurePath('<folder>', '<stub>')}, from the root of the project."
"path", help=f"Path of the stub to test in format {Path('<folder>', '<stub>')}, from the root of the project."

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 don't have enough knowledge to say whether Path or PurePath is better or if they are the same. PurePath lacks the logic that interacts with the filesystem, and in this case, the < and > characters are illegal in Windows. So, I would balk at using Path because I would be worried that Path might try to validate the "path" and throw an error.

That same logic could be extended to suggest that os.path.sep is safer than PurePath.

Nevertheless, I don't know: I am speculating.

Path does work on my Windows system, though.

(.venv) C:\apps\astToolkit>py
Python 3.13.3 (tags/v3.13.3:6280bb5, Apr  8 2025, 14:47:33) [MSC v.1943 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from pathlib import Path
>>> {Path('<folder>', '<stub>')}
{WindowsPath('<folder>/<stub>')}
>>> 

@Avasam
Copy link
Collaborator

Avasam commented May 5, 2025

Now that the underlying issue of #13936 has been fixed in #13943, we can decide whether this is something we want on its own merit purely for documentation/visual purposes.

I'll quote my comment at #13936 (comment)

Either slashes should work fine. (even prepended by ./ or .\ due to tab-completion)
[...]
I generally avoid backslashes in doc/helpers because they can be very annoying to copy (copying a path on windows and deduplicating \\ is so annoying. So is copying a backslash somewhere and having it accidentally escape a character)

I like the simplicity of just using a simple / for any documentation, comment, help message, etc.

@AlexWaygood
Copy link
Member

I agree with @Avasam. / slashes are easier to write and should work fine on Windows generally. Windows users should be used to seeing them as path separators (I was when I was a Windows user!). If #13936 is now fixed, I'd prefer to leave things as they are and not make this change.

@hunterhogan
Copy link
Contributor Author

hunterhogan commented May 5, 2025

[edit: I should not write when I'm experiencing excessive pain.]

Thank you for fixing the hardcoded os.path.sep issue.

@AlexWaygood
Copy link
Member

[edit: I should not write when I'm experiencing excessive pain.]

I'm very sorry to hear that, and I'm sorry if I was tetchy earlier. We're of course grateful for you taking the time to contribute! We're just not yet convinced that this change will be a net benefit to everybody running this script.

I hope you get well soon.

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.

Running "tests/runtests.py" on Windows
3 participants