-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
base: main
Are you sure you want to change the base?
Conversation
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>')}.") |
There was a problem hiding this comment.
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)
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." |
There was a problem hiding this comment.
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
"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." |
There was a problem hiding this comment.
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>')}
>>>
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)
I like the simplicity of just using a simple |
[edit: I should not write when I'm experiencing excessive pain.] Thank you for fixing the hardcoded |
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. |
Clarify the help message for the 'path' argument in the
runtests.py
script to reflect the correct format usingPurePath
. This change ensures that users receive accurate guidance on how to specify the path, improving usability on different operating systems.Fixes #13936