-
-
Notifications
You must be signed in to change notification settings - Fork 30.7k
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
A more compact pathlib summary #127911
base: main
Are you sure you want to change the base?
A more compact pathlib summary #127911
Conversation
72669dd
to
dc2c6ae
Compare
- Removed class names and parameters from the summary - Used parentheses uniformly to indicate methods - Shortened some of the summary descriptions - Move "exceptions" to the end
dc2c6ae
to
ab7fc3a
Compare
-------------------------------------------------------------------------------------------------------------------------- | ||
:meth:`copy() <Path.copy>` Copy this file or directory tree to the given *target* | ||
:meth:`copy_into() <Path.copy_into>` Copy this file or directory tree into the given *target_dir* | ||
:meth:`rename() <Path.rename>` Rename this file or directory to the given *target* |
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.
:meth:`rename() <Path.rename>` Rename this file or directory to the given *target* | |
:meth:`rename() <Path.rename>` Rename this file or directory to the given *target*. Raises an exception if the *target* already exists. |
I think is clarification would be good.
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.
We don't need to add this second sentence. This table is meant to help people find what they need, and then go read the full description. There are many reasons why a method could raise an exception. This doesn't need to be mentioned in the summary.
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.
Should we not differentiate between rename/replace as they share the first sentence which may be confusing?
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.
Sorry, now I see what you mean. Reading the full descriptions, it's even more subtle, and OS-specific. I would go with:
rename: Rename this path to the given target if possible.
replace: Rename this path to the given target unconditionally.
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 mind which one you go with both mine and yours get the point across.
This is a reduced summary table, to help with smaller screens and to make the content more scannable.
Compared to #126342, I've:
📚 Documentation preview 📚: https://cpython-previews--127911.org.readthedocs.build/
Link to the pathlib preview: https://cpython-previews--127911.org.readthedocs.build/en/127911/library/pathlib.html#summary