Skip to content

Conversation

@refack
Copy link

@refack refack commented Jan 20, 2025

Although I have much experience with POSIX, my home turf is WIN32 and here I had to read the text several times until the penny dropped. I hope my tiny change makes it easier to understand.


📚 Documentation preview 📚: https://cpython-previews--129103.org.readthedocs.build/

@ghost
Copy link

ghost commented Jan 20, 2025

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news awaiting review labels Jan 20, 2025
@picnixz
Copy link
Member

picnixz commented Jan 20, 2025

I think the current wording is fine. Modes w+ and wb+ "open and truncate" the file and I don't see how it can be misinterpreted. It's not jus an "updating mode" (which by the way I don't know the terminology of), but it's an "open-and-truncate" action.

@picnixz picnixz added the pending The issue will be closed if no feedback is provided label Jan 20, 2025
@refack
Copy link
Author

refack commented Jan 20, 2025

Hello @picnixz, and thank you for the feedback.

I think the current wording is fine. Modes w+ and wb+ "open and truncate" the file and I don't see how it can be misinterpreted. It's not jus an "updating mode" (which by the way I don't know the terminology of), but it's an "open-and-truncate" action.

I was trying to be as succinct as possible, so I used the definition of updating used two lines above, i.e. updating (reading and writing) . But this is just my attempt at making this a little bit simpler for ASL and PSL (POSIX as Second Language) people.

``'+'`` open for updating (reading and writing)
========= ===============================================================
The default mode is ``'r'`` (open for reading text, a synonym of ``'rt'``).
For updating modes, ``'w+'`` and ``'w+b'`` will truncate the file, while
``'r+'`` and ``'r+b'`` open the file with no truncation.

To be clear, IMHO the current wording is obviously correct. It's just that I felt there was not enough emphasis on the fact that r+ and w+ are essentially the same except for the implicit truncation nuance (why 'implicit' because not w nor r have explicit 'truncate' connotations)

@picnixz
Copy link
Member

picnixz commented Jan 20, 2025

Ah I see the nuance. Would this be retained in the following formulation:

Modes 'w+' and 'w+b' open and truncate the file, while modes
'r+' and 'r+b' open the file but do not truncate the file.

I think we should definitely keep the fact that w+ and w+b actually open and truncate the file (namely, they do two things).

@refack
Copy link
Author

refack commented Jan 20, 2025

Ah I see the nuance. Would this be retained in the following formulation:

Modes 'w+' and 'w+b' open and truncate the file, while modes
'r+' and 'r+b' open the file but do not truncate the file.

I think we should definitely keep the fact that w+ and w+b actually open and truncate the file (namely, they do two things).

LGTM

@picnixz
Copy link
Member

picnixz commented Jan 21, 2025

cc @python/proofreaders to decide whether this new formulation is preferred or not (I don't have a strong preference and I wouldn't mind the status quo; it's more for others and to see if other people think it would be clearer).

(Sorry for the editorial board ping)

@picnixz picnixz removed the pending The issue will be closed if no feedback is provided label Jan 21, 2025
@refack
Copy link
Author

refack commented Jan 21, 2025

Actually I'd make a tiny edit, based on "escalating significance" to something like.

Modes 'r+' and 'r+b' open file for updating while modes
'w+' and 'w+b' open and truncate the file.

But this is a tiny tiny nitpick.

@bskinn
Copy link
Contributor

bskinn commented Jan 21, 2025

Nitpick:

(why 'implicit' because not w nor r have explicit 'truncate' connotations)

But w does have an explicit 'truncate' meaning -- it's in both the table and paragraph immediately above.


In any event, I'm okay with either wording. I'm pretty deeply familiar with Python and POSIX, though, so I'm probably a poor person to ask about the clarity of either.

That said, more generally, rather than targeted edits to this small paragraph, I think the broader section could be revised more extensively for clarity. I don't have time at the moment to draft anything (I likely could somewhat soon, if desired), but at least:

  • The table should really be separated into three parts: (1) r/w/x/a, (2) t/b, and (3) +.
    • (1) core file interaction modes
    • (2) data interface type
    • (3) converts whatever mode of (1) into update mode, adding writing to r and adding reading to the others
  • The text should be revised to explain the mode string "syntax" more clearly; along the lines of:
    • If mode is specified, it has to have one of (1) as its first character; if mode is not specified, it defaults to r
    • Regardless of the first character, the second character MAY be the + of (3), which converts to 'update mode' (and then explain). By default update mode is disabled.
    • The final character is then (2); if omitted, the default is t (with associated explanation)
  • It might even make sense to create the small Backus-Naur grammar for mode.

@mwichmann
Copy link

I'll toss in a little nit as well: the function is open and the introductory parts make it clear that all the modes lead to opening the file, so it's not necessary to repeat "open" so many times in the following text.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I'm not feeling the value of the proposed rewording, sorry. I propose to close the PR without merging.

Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

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

Thanks @refack for making your first PR to CPython. I agree with @gvanrossum that we should keep this documentation as is since your proposed wording adds a bit of complexity by combining the sentences.

I'm going to close this PR. I do encourage you to keep contributing to the documentation especially if you see an open issue which you could help improve. Thanks. 🌞

@willingc willingc closed this Jan 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

7 participants