PEP 544: Replace broken link to Zope documentation

[aphedges]

I've been reading through some PEPs to better educate myself about Python, and I found some room for improvement in PEP 544 "Protocols: Structural subtyping (static duck typing)". I made three separate commits to make it easier to accept/reject them individually:

• PEP 544: Fix errors in class object example: While reading through the PEP, I noticed that the comments for type errors were reversed in the third code block in the "Type[] and class objects vs protocols" section. I ran the example through mypy 0.931 using Python 3.10.2 to verify I was correct, and it found some other errors that I needed to fix before I could get the expected error:

  $ mypy example.py
  example.py:11: error: Incompatible types in assignment (expression has type "Type[C]", variable has type "ProtoA")
  example.py:12: error: Incompatible types in assignment (expression has type "Type[C]", variable has type "ProtoB")
  Found 2 errors in 1 file (checked 1 source file)
  $ mypy example.py
  example.py:12: error: Incompatible types in assignment (expression has type "Type[C]", variable has type "Type[ProtoB]")
  Found 1 error in 1 file (checked 1 source file)

  If these changes are wrong, then there is a bug in mypy that needs fixing.

• PEP 544: Replace broken link to Zope documentation: The documentation links to https://www.zope.dev/zope.interface/adapter.html, but it appears to have been broken for quite a while, possibly not too long after it was first added in ea2cd15. I replaced it with https://web.archive.org/web/20160802080957/https://docs.zope.org/zope.interface/adapter.html, which was the most recent version available. I also found https://zopeinterface.readthedocs.io/en/latest/adapter.html, which contains very similar content, but I personally believe the Wayback Machine link is safer because it is less likely to change.

  The change went over the line length limit, but linting checks still pass. Do I need to manually wrap the markup?

• PEP 544: Rename confusing identifiers in example: I found that the second code block in the "Support isinstance() checks by default" section was confusing because of the multiple uses of x as an identifier. It is obviously subjective about whether this is confusing, but changing some of the usages seems like a safe change.

I decided to be a completionist and run all other Python code blocks through mypy to check for correctness. I intentionally skipped the "Rejected/Postponed Ideas" section because they probably shouldn't work. I also skipped the zope and non-Python code because those are more difficult to check. I for the most part found every example is correct, but I found two problems worth bringing up:

• The first code block in the "Explicitly declaring implementation" section includes a line with the comment # Error, no default implementation, even though it passes mypy checks and does not error when the line is executed. For context, here is a modified minimal version of the code block that can run without unrelated errors:

  from abc import abstractmethod
  from typing import Protocol
  
  class PColor(Protocol):
      @abstractmethod
      def draw(self) -> str:
          ...
  
  class BadColor(PColor):
      def draw(self) -> str:
          return super().draw()  # Error, no default implementation
  
  bad = BadColor()
  bad.draw()

  I believe that either the comment should not be here or I should be writing up an issue for mypy. Alternatively, I might not be using the write mypy settings to detect this, but the --strict flag doesn't help, so I'm not sure there is a lot more to search there.

• I had to do a lot of cleanup to the code blocks to run them through mypy. Many would fail to run as Python scripts. Most of these were due to missing or incomplete imports, but others wouldn't even compile due to loose syntax. I am not proposing that the Python code in PEPs should be automatically validated, but given that the documentation for typing.Protocol says to "See PEP 544 for details.", this PEP is somewhat of an extension of the main documentation. I checked PEP 1 and PEP 12 to see if they have any guidance on this topic. They do not specify about code example validity, so I'm not sure if this is something I should try to fix.

[JelleZijlstra]

This change is incorrect. We discussed this in #2332 recently.

[aphedges]

Hahaha! I found this issue last week, but I didn't get around to making a PR until after #2332.

Thanks for letting me know that this is a long-outstanding bug in mypy: python/mypy#4536. That issue also links to #1231, so this is at least the third attempt to fix this example. When it makes it into the typing documentation, there should probably be some improved wording.

[aphedges]

Also, I should have searched for this first before trying to fix it. I'm sorry for not doing so. It would have saved us both some time.

[JelleZijlstra]

This change seems gratuitous. The previous x is in a different example.

[aphedges]

I was specifically referring to confusion with the use of x later in the code block, but I did think this change would be deemed unnecessary.

[JelleZijlstra]

Thanks for your contribution! In general we try to avoid changes to old, long-accepted PEPs. We are trying to get more up-to-date typing documentation in typing.readthedocs.io (https://github.com/python/typing).

[aphedges]

@JelleZijlstra, thanks for the review!

I understand that you try to avoid changes to old PEPs, but I had assumed this was different because it's essentially typing documentation. It's good to know there will be a more dynamic place for typing documentation, even if it isn't ready yet.

I planned to make additional PRs for problems I found with other PEPs, so I'll make sure to limit them to problems like broken links. Do you think it would be a good idea to codify in CONTRIBUTING.rst what kinds of changes to old PEPs should be allowed and which should not? I can see there is a PR in progress that says to generally avoid fixing old spelling errors, for instance.

I will edit this PR to remove the rejected commits and edit the title and description accordingly. I'll re-request a review once those actions have been taken.

[CAM-Gerlach]

We are trying to get more up-to-date typing documentation in typing.readthedocs.io (https://github.com/python/typing).

Maybe something explicit like what we do for the packaging specs would be useful here, i.e. having that site be the canonical location of the living documentation, have the main PEPs link to it, and then new PEPs, if accepted, would update it, while changes deemed trivial/non-substantive would not need to go through the PEP process?

I planned to make additional PRs for problems I found with other PEPs, so I'll make sure to limit them to problems like broken links. Do you think it would be a good idea to codify in CONTRIBUTING.rst what kinds of changes to old PEPs should be allowed and which should not? I can see there is a PR in progress that says to generally avoid fixing old spelling errors, for instance.

See #2338 , which adds a note to this effect. Maybe we can go with the suggestion there to elevate it to a top-level section, and have it more generally discuss the types of contributions that are helpful?

[JelleZijlstra]

Maybe something explicit like what we do for the packaging specs would be useful here, i.e. having that site be the canonical location of the living documentation, have the main PEPs link to it, and then new PEPs, if accepted, would update it, while changes deemed trivial/non-substantive would not need to go through the PEP process?

I think that would be a good end state, but we're not even close to being there yet.

See #2338 , which adds a note to this effect. Maybe we can go with the suggestion there to elevate it to a top-level section, and have it more generally discuss the types of contributions that are helpful?

Yes, I think that would be good.

[JelleZijlstra]

Thanks, this looks good! Planning to leave this for a day or so just in case the PEP authors have feedback.

[CAM-Gerlach]

I think that would be a good end state, but we're not even close to being there yet.

Well, to be fair, we're not fully there with packaging either, as only about half the PEPs actually have spec pages there at present—I've meant to help finish that, but time is always a limiting factor.

Yes, I think that would be good.

In work, thanks.

[aphedges]

Maybe something explicit like what we do for the packaging specs would be useful here, i.e. having that site be the canonical location of the living documentation, have the main PEPs link to it, and then new PEPs, if accepted, would update it, while changes deemed trivial/non-substantive would not need to go through the PEP process?

That came to mind, but I didn't suggest that because there are no copies of the PEPs on the new typing site yet to link to.

See #2338 , which adds a note to this effect. Maybe we can go with the suggestion there to elevate it to a top-level section, and have it more generally discuss the types of contributions that are helpful?

That is the PR that I was referring to, which probably should have linked. I think that having a more general section on what types of contributions are helpful would be a good idea. There is a "Before writing a new PEP" section, but there is no guidance on what edits are acceptable for existing, accepted PEPs.

[CAM-Gerlach]

One link suggestion, per the recently revised PEP 12.

Sidenote: A lot of PEPs don't strictly keep to the nominal line length limit, and actually enforcing it would require a ton of nitpicking. Personally, I'd be strongly in favor of recommending (or at least allowing) the one sentence per line approach for prose text in new PEPs, since any editor can soft-wrap text (far more than that can intelligently hard wrap it, without breaking the syntax), it makes reading, writing, editing and reviewing text so much simpler and less painful, it neatly sidesteps the one or two space after a period debate while being easier to parse visually and by machine, and leads to much less noisy and more meaningful diffs, Git blame and more. But I digress...

[aphedges]

One link suggestion, per the recently revised PEP 12.

Implemented.

Sidenote: A lot of PEPs don't strictly keep to the nominal line length limit, and actually enforcing it would require a ton of nitpicking. Personally, I'd be strongly in favor of recommending (or at least allowing) the one sentence per line approach for prose text in new PEPs, since any editor can soft-wrap text (far more than that can intelligently hard wrap it, without breaking the syntax), it makes reading, writing, editing and reviewing text so much simpler and less painful, it neatly sidesteps the one or two space after a period debate while being easier to parse visually and by machine, and leads to much less noisy and more meaningful diffs, Git blame and more. But I digress...

That's an interesting idea. I'm used to seeing conventions that either enforce wrapping or do none at all, but one sentence per line definitely results in better diffs than wrapping after N characters.

[CAM-Gerlach]

LGTM, thanks @aphedges

[CAM-Gerlach]

Implemented.

Thanks! Just FYI for the future, instead of manually applying reviewer-suggested changes yourself, with GitHub suggestions you can simply click the Commit button for single changes, or on the Files tab, Add to batch and then the Commit for multiple, to apply them directly. This is less work for you, ensures they get applied correctly, and auto-resolves the reviewer comments rather than the reviewer having to verify that. Plus, it credits the suggester for their changes (which isn't all that important to me, but does matter to some).

That's an interesting idea. I'm used to seeing conventions that either enforce wrapping or do none at all, but one sentence per line definitely results in better diffs than wrapping after N characters.

One Sentence Per Line (aka semantic line breaks) is an increasingly popular convention for prose like this (its been used for years in most of the reST/myST, website and docs-related repos I'm involved in, as well as on most others for Readmes, Contributing Guides, etc. Its the standard for the AsciiDoc docs format and some others. One Paragraph Per Line (what I assume you mean by "no" breaks) is the other alternative, having the main advantage that it is somewhat more "natural" for users to type, but on a repo like this with a heavy amount of detailed editing and reviewing, and a fairly experienced contributor audience, the benefits of OSPL over it for comments, suggestions and diffs/blame are substantial, while also having most of the benefits of hard breaks, and a few unique to it.

But again, I digress, sorry...

[aphedges]

Thanks! Just FYI for the future, instead of manually applying reviewer-suggested changes yourself, with GitHub suggestions you can simply click the Commit button for single changes

I am aware of that feature, and I usually use it, but I decided to go with more descriptive wording than you proposed.

One Sentence Per Line (aka semantic line breaks) is an increasingly popular convention for prose like this (its been used for years in most of the reST/myST, website and docs-related repos I'm involved in, as well as on most others for Readmes, Contributing Guides, etc. Its the standard for the AsciiDoc docs format and some others. One Paragraph Per Line (what I assume you mean by "no" breaks) is the other alternative, having the main advantage that it is somewhat more "natural" for users to type, but on a repo like this with a heavy amount of detailed editing and reviewing, and a fairly experienced contributor audience, the benefits of OSPL over it for comments, suggestions and diffs/blame are substantial, while also having most of the benefits of hard breaks, and a few unique to it.

I was not aware, so thanks for explaining. I usually write with Markdown, and GitHub's renderer takes line breaks into account, so One Sentence Per Line seems to be better for some markup languages than others.

[CAM-Gerlach]

I am aware of that feature, and I usually use it, but I decided to go with more descriptive wording than you proposed.

Ah, gotcha, nice 👍—I didn't realize that from your reply until I examined your commit carefully.

I usually write with Markdown, and GitHub's renderer takes line breaks into account, so One Sentence Per Line seems to be better for some markup languages than others.

That's only for issues/PRs/comments (like here).
It renders Markdown files committed the repo normally per the Markdown spec.

[aphedges]

Ah, gotcha, nice 👍—I didn't realize that from your reply until I examined your commit carefully.

Sorry. Rereading my comments, I see I should have been more explicit.

[benji-york]

Using an archive link is probably a good idea here, but if the current version of the document is of interest, it is at https://github.com/zopefoundation/zope.interface/blob/master/docs/adapter.rst

[aphedges]

Thanks for the suggestion, but I did mention the deployed version of that page in the PR description:

I also found https://zopeinterface.readthedocs.io/en/latest/adapter.html, which contains very similar content