PEP 675: Switch to the name LiteralString

[pradeep90]

We discussed a lot of alternatives on typing-sig over the last couple of months.

Literal[str] was pretty strongly rejected. Every candidate had some objection, but StringLiteral/LiteralString seemed to have quite a lot of support.

So, we decided to go in favor of LiteralString.

cc @gbleaney

[JelleZijlstra]

Thanks! I have a few small comments.

[JelleZijlstra]

Could instead add a note about adding LiteralString to typing_extensions.

[pradeep90]

Ok, I've added a line. Lmk if that looks fine.

[JelleZijlstra]

Thanks, looks good

[CAM-Gerlach]

Thanks @pradeep90 ! I just had one syntax fix and a few minor copyediting/proofreading suggestions.

[CAM-Gerlach]

Suggested change

	<inferring_literal_string>`_ section. Inferring the type ``LiteralString``
	<inferring_literal_string_>`_ section. Inferring the type ``LiteralString``

Syntax fix

[pradeep90]

I swear RST is the least memorizable syntax I've ever come across. I can never recall how to link to something :| Thank goodness we have Markdown elsewhere.

(Why didn't the RST pre-commit checks catch this?)

[CAM-Gerlach]

I swear RST is the least memorizable syntax I've ever come across. I can never recall how to link to something :| Thank goodness we have Markdown elsewhere.

Heh, no worries; I've had multiple PEP editors (including the writer of the new build system) think this wasn't required, due to the different rules for implicit references vs. explicit reference roles. There are a number of things that could be more intuitive and user-friendly about RST, links being the worst offender, but with Sphinx it is far more powerful than Markdown when it comes to its functionality for documentation and things like the PEP repo; a lot of what we do here simply wouldn't be possible without RST's extensible roles, directives and structured style.

(Why didn't the RST pre-commit checks catch this?)

There isn't currently a check for this, but I could write a new one. However, assuming PEP-0676 is accepted, once the new Sphinx build system is the default, you can just use the standard :ref: role to link sections, which doesn't have this problem and also has the section name as its default inline text, so instead of

`Inferring LiteralString <inferring_literal_string_>`_

you could just do

:ref:`inferring_literal_string`

[CAM-Gerlach]

Suggested change

	<https://mail.python.org/archives/list/typing-sig@python.org/thread/VB74EHNM4RODDFM64NEEEBJQVAUAWIAW/>`_. Some
	notable alternatives were:
	<https://mail.python.org/archives/list/typing-sig@python.org/thread/VB74EHNM4RODDFM64NEEEBJQVAUAWIAW/>`_.
	Some notable alternatives were:

Formatting nitpick

[pradeep90]

Can we add a lint rule for this? I like the other RST checks as part of the pre-commit tests. I don't know what is wrong here. (I just use fill-paragraph in Emacs.)

[CAM-Gerlach]

Its just a formatting nitpick; doesn't actually affect anything in the rendered document. For whatever reason fill-paragraph is not adding the break before Some despite it being at the end of a sentence and it being far over the maximum column width, either because it is getting confused about RST syntax or it doesn't have two spaces after periods. Allowing One Line Per Sentence fur future PEPs instead of just old-fashioned hard breaks would fix all of these issues and much more, and make reviews much easier and diffs much cleaner, but I've yet to formally propose it since I've been focused on more important things first.

[CAM-Gerlach]

Suggested change

	+ ``LiteralStr``: This is shorter than ``LiteralString`` but looks
	+ ``LiteralStr``: This is shorter than ``LiteralString``, but looks

Missing comma

[pradeep90]

No, it's not an independent clause.

[CAM-Gerlach]

Looks like this was not applied.

[pradeep90]

Yes, and I explained why I did not apply it: "looks weird to the authors" is not an independent clause. We don't add a comma before "but" in those cases.

[CAM-Gerlach]

It wasn't syntactically required, but it sounded better to me as a copyeditor in terms of how I read it. Reading it again, though, and getting a second opinion from my sister (a professional writer, whom I sometimes copyedit for), I agree you're right that it sounds fine without it.

[CAM-Gerlach]

Suggested change

	such as ``"foo"`` but also that of expressions composed from
	such as ``"foo"``, but also that of expressions composed from

Missing comma

[pradeep90]

Same - not an independent clause.

[CAM-Gerlach]

This too

[pradeep90]

Same as above, I explained why I did not apply it.

[CAM-Gerlach]

Actually, I made an mistake here that my sister spotted when giving me a second opinion on this—such as ``"foo"`` should be set off by commas, rather than just having only a single comma after it. I'll make another suggestion to that effect.

[CAM-Gerlach]

Suggested change

	demand for ``LiteralBytes``, so we decided not to include it in this
	PEP. Others may however consider it in future PEPs.
	demand for ``LiteralBytes``, so we decided not to include it here.
	It could, however, be added in a future PEP.

Improve phrasing and fix repetition

[pradeep90]

Eh, I prefer the parallelism of the original.

[CAM-Gerlach]

This wasn't applied and went stale; I remade it.

[CAM-Gerlach]

FYI, @pradeep90 , looks like a few suggestions were not applied. Remember that you can and generally should apply suggestions directly via the GitHub UI, as intended, which automatically commits it and auto-resolves the linked comments. Otherwise, its easy to miss something or make a mistake, and reviewers have to manually verify that each suggestion was actually applied. Here's my advice on your last PR for how to do that:

FYI, if you go to the files tab of the PR, you can click "Add to batch" on each suggestion and then use the "Commit" button at the top to commit them, preferably with an informative message. This commits the suggestions directly to your branch (which you then can pull, etc. locally) in one click, marks and auto-resolves the linked review comments, and gives the suggestion contributor attribution (which isn't a bit deal to me, but matters to some people). This is much easier and faster than applying them manually, avoiding extra work for you and the chance of making a mistake or missing a suggestion, and is also easier for reviewers, as its clear what suggestions were applied and in what commits.

[CAM-Gerlach]

Suggested change

	demand for ``LiteralBytes``, so we decided not to include it in this
	PEP. Others may, however, consider it in future PEPs.
	demand for ``LiteralBytes``, so we decided not to include it here.
	It could, however, be added in a future PEP.

This suggestion (to "Improve phrasing and fix repetition") went stale because changes were pushed after it was made but before it was applied, so I re-made it. As a general rule, its good to always apply suggestions before making other, more substantive changes.

[JelleZijlstra]

Pradeep commented on this and a few other suggestions why he preferred not to apply them. I'd like to leave that decision to him as the author.

From my perspective this is ready to merge.

[pradeep90]

No, I did not miss it. I replied to the original suggestion with the reason why I did not apply it (I preferred the parallelism of the original version).

Edit: Oops, I replied to the original comment before I saw Jelle's comment.

[CAM-Gerlach]

I now see commas were added around however, which was the really critical change; the rest I yield to your judgement.

[pradeep90]

FYI, @pradeep90 , looks like a few suggestions were not applied.

I resolved the suggestions that I applied and replied to the rest with the reason why not. Namely, we don't add a comma before but when the latter clause is not independent.

Remember that you can and generally should apply suggestions directly via the GitHub UI, as intended, which automatically commits it and auto-resolves the linked comments. ... Here's my advice on your last PR for how to do that:

I'm aware of the Github UI feature, @CAM-Gerlach.

reviewers have to manually verify that each suggestion was actually applied.

I disagree that there is extra work for reviewers because the applied suggestions are marked resolved either way. The main difference is whether the author prefers the workflow. I don't prefer it, but might try it in the future.

[CAM-Gerlach]

@JelleZijlstra @pradeep90 Your replies to my comments did not show up for me until I'd finished making mine and reloaded the page, despite your top-level comment appearing just fine. Its happened to me before, probably due to GitHub caching; I apologize for the confusion. I will reply to them now, and then your top-level comment here.

[CAM-Gerlach]

I realized I made a mistake on one of my previous suggestions—the whole "such as" phrase should be set off by commas. Otherwise, I agree this is fine as-is, having now seen and carefully considered your replies.

[CAM-Gerlach]

It wasn't syntactically required, but it sounded better to me as a copyeditor in terms of how I read it. Reading it again, though, and getting a second opinion from my sister (a professional writer, whom I sometimes copyedit for), I agree you're right that it sounds fine without it.

[CAM-Gerlach]

Actually, I made an mistake here that my sister spotted when giving me a second opinion on this—such as ``"foo"`` should be set off by commas, rather than just having only a single comma after it. I'll make another suggestion to that effect.

[CAM-Gerlach]

I now see commas were added around however, which was the really critical change; the rest I yield to your judgement.

[CAM-Gerlach]

Just to preface, as I've commented before, you are an excellent writer and should be rightly proud of it. I hope nothing I've said, or will say, has implied otherwise.

I disagree that there is extra work for reviewers because the applied suggestions are marked resolved either way. The main difference is whether the author prefers the workflow. I don't prefer it, but might try it in the future.

I cannot speak for everyone, but I can categorically state that at least for myself as a PEP editor (who spends many hours of my volunteer time each week applying my background and experience as a technical writer and copyeditor toward helping PEP authors like yourself fix their typos and syntax errors, improve the clarity and quality of their prose, use appropriate RST constructs for effective, attractive rendered output, and conform to the formatting and technical requirements here), it makes my job a lot easier to know that the suggestions I took the time to make actually got applied (or declined, as the case may be), rather than redone manually, which I then have to manually verify.

Furthermore, on more personal note, it is honestly pretty demotivating to me when I'm spending hours of my time carefully copyediting and proofreading a PR, and taking the time to actually test out and make the suggested changes, to know that someone is going to knowingly throw away my work and spend all the time redoing it themselves instead, and then I have to spend yet more time manually re-reviewing it—not only because in taking the time to make a suggestion I'm creating more work for myself, but also that I'm creating more (needless) work for the author manually fixing it.

I'm baffled by why someone would actively refuse to take advantage of this feature in cases where it was appropriate—could you explain? It is hard for me to understand why someone would knowingly choose to, for each and every comment, read it, identify the affected lines, scroll to them, make the change, and then mark the comment as resolved, when they could accomplish exactly the same thing and avoid all this needless make-work by pressing a single button while scrolling through the source if the change looks good to them.

Other than a few technical details, those commits are identical to what you'd make locally (which can easily be fixed up), and they all get squashed anyway when we merge. If you don't like how it gives us co-author credit on the relevant commits, that's easy to fix by running git commit --amend and deleting the Co-authored-by line, or we can even remove it for you when merging. If there's something else, I'd be curious to hear it and happy to help brainstorm a solution.

[pradeep90]

I cannot speak for everyone, but I can categorically state that at least for myself as a PEP editor (who spends many hours of my volunteer time each week applying my background and experience as a technical writer and copyeditor toward helping PEP authors like yourself fix their typos and syntax errors, improve the clarity and quality of their prose, use appropriate RST constructs for effective, attractive rendered output, and conform to the formatting and technical requirements here),

Thank you for all the time you've put in! I do appreciate it, especially when it's in the form of frameworks or lint rules that can scale to lots of users (and be reused in other projects).

I'm baffled by why someone would actively refuse to take advantage of this feature in cases where it was appropriate—could you explain? It is hard for me to understand why someone would knowingly choose to, for each and every comment, read it, identify the affected lines, scroll to them, make the change, and then mark the comment as resolved, when they could accomplish exactly the same thing and avoid all this needless make-work by pressing a single button while scrolling through the source if the change looks good to them.

It's a matter of taste. There's a reason why we don't all use the same editor :)

Since you asked, a big reason is that, instead of literally applying a reviewer's suggestion, I like to take the general idea behind it and look for other places in my document where I can use the idea. For example, when you pointed out the lack of commas around "such as" when it was a non-restrictive clause, I looked for and found a bunch of other such phrases in the PEP. I may also want to apply only part of a suggestion, such as the commas around "however". Let me know if that makes sense.

(If you suggest that I auto-accept the suggestions, pull the commit, and then search for similar mistakes, that frankly seems like way more work :) )

If you don't like how it gives us co-author credit on the relevant commits

That never even crossed my mind.

it is honestly pretty demotivating to me when I'm spending hours of my time carefully copyediting and proofreading a PR, and taking the time to actually test out and make the suggested changes, to know that someone is going to knowingly throw away my work and spend all the time redoing it themselves instead, and then I have to spend yet more time manually re-reviewing it

It looks like we're approaching it from different points of view. When I review code or text, I think of the "suggested changes" feature more as a way to get across precisely what I mean than as a way to save the author a few seconds. I expect that they will spend way more time anyway on my more substantive suggestions. And I don't manually re-review the applied change because I trust that the author cares enough to apply it correctly.

These assumptions may or may not hold for you on an open-source repo like this, but I hope it can make it feel less demotivating. I'm not at all trying to throw away your hard work! (I spent two hours reviewing a PR today - and I'm still not done - so I do sympathize with your efforts.) And I ensure that all suggestions are either resolved or replied to before I request re-review - that's how I know I'm done. I hope that helps.

[CAM-Gerlach]

LGTM, thanks @pradeep90 ! I'll reply to your thoughtful message in a little bit, but I wanted to get this merged for you before a long meeting I have just now. Thanks for your patience!

[CAM-Gerlach]

Sorry, I was a little tired and grumpy old CAM last night and should have gone to bed instead of responding late at night (like I did for the message above).

Thank you for all the time you've put in! I do appreciate it, especially when it's in the form of frameworks or lint rules that can scale to lots of users (and be reused in other projects).

Thanks! That's one of the things I most enjoy doing, and developing/maintaining the linting infra and related stuff is one of the main things I've taken on around here.

It's a matter of taste. There's a reason why we don't all use the same editor :)

No, we all know that ${EDITOR} is the best and everyone who thinks otherwise is just wrong 😂

I like to take the general idea behind it and look for other places in my document where I can use the idea. [...] Let me know if that makes sense.

Actually, I love that! When I used to work as a writing tutor, and even in a number of other things, I actually would often show writers my suggested changes and explain the motivation for them, but then have them revise their writing themselves, so that they learn more from the experience and understand how and why to incorporate it into their future writing, while being able to retain their own unique author's voice, instead of just sounding like me all the time. While its a little different when it comes to technical writing and documentation, which I mostly work on nowadays; especially for an excellent writer like yourself its great that you're still taking the time to think about that. There's always something to learn from others (I certainly was reminded of that above).

It looks like we're approaching it from different points of view. When I review code or text, I think of the "suggested changes" feature more as a way to get across precisely what I mean than as a way to save the author a few seconds. I expect that they will spend way more time anyway on my more substantive suggestions.

Actually, now that I'm not so tired and cranky, I think you and I have a lot more in common than either of us initially realized. Yeah, in the heat of the moment I didn't really think of it so much that way, but that is really true, even if the suggestion was not applied. I can think of a lot of times (from both the perspective of a reviewer and an author) where a ton of time was wasted going back and forth with a reviewer when it wasn't clear to the author exactly what they wanted, way more than either making or manually applying a specific suggestion. It tends to be that I do make a lot of small suggestions here, given the role of a PEP editor, but even still it is generally a few larger ones, if I have them (or perhaps like Jelle's here) that take more time than all the smaller ones.

And I don't manually re-review the applied change because I trust that the author cares enough to apply it correctly.

Yeah, I guess I'm just not a very trusting person that others won't make mistakes (nor do I trust myself in that aspect 😆 ), even when they are clearly very competent like yourself, due to working with a number of folks with less experience, knowledge or skill level in the past for various things. I probably should be more trusting of others...I just have a big fear of not noticing and pointing out a significant mistake and make myself look stupid, to the point where I can be afraid to say positive remarks about something for fear someone else will point out a flaw.

These assumptions may or may not hold for you on an open-source repo like this, but I hope it can make it feel less demotivating. I'm not at all trying to throw away your hard work! (I spent two hours reviewing a PR today - and I'm still not done - so I do sympathize with your efforts.) And I ensure that all suggestions are either resolved or replied to before I request re-review - that's how I know I'm done. I hope that helps.

💯