Skip to content
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

PEP 495: Fix incorrect gap image #2965

Merged
merged 1 commit into from
Jan 22, 2023
Merged

PEP 495: Fix incorrect gap image #2965

merged 1 commit into from
Jan 22, 2023

Conversation

bxparks
Copy link
Contributor

@bxparks bxparks commented Jan 16, 2023

The SVG image in the "Mind the Gap" section (pep-0495-gap.svg) is incorrect. It is essentially the same as the the SVG image in the "In the Gap" section (pep-0495-fold.svg). This is a bug that dates back to the original commit on 2015-08-30 (814d372) that uploaded the both the PNG and SVG versions of the Fold and Gap images. The PNG version of the Gap image is correct, but the SVG version is incorrect.

I believe this bug was invisible prior to 2022-04-23 because PEP 495 previously referenced the PNG versions of the images (pep-0495-gap.png, pep-0495-fold-2.png). Commit 28a5eb3 deleted the PNG images and changed the links in PEP 495 to point to the SVG versions.

The replacement SVG image attached in this PR is a manual recreation of the PNG version of the Gap image that was deleted on 2022-04-23. It is not exactly the same, but I think it is close enough.

Fixes #2959

The SVG image in the "Mind the Gap" section (pep-0495-gap.svg) is incorrect. It
is essentially the same as the the SVG image in the "In the Gap" section
(pep-0495-fold.svg). This is a bug that dates back to the original commit on
2015-08-30 (814d372) that uploaded the both
the PNG and SVG versions of the Fold and Gap images. The PNG version of the Gap
image is correct, but the SVG version is incorrect.

I believe this bug was invisible prior to 2022-04-23 because PEP 495 previously
referenced the PNG versions of the images (pep-0495-gap.png,
pep-0495-fold-2.png). Commit 28a5eb3 deleted
the PNG images and changed the links in PEP 495 to point to the SVG versions.

The replacement SVG image attached in this PR is a manual recreation of the PNG
version of the Gap image that was deleted on 2022-04-23. It is not exactly the
same, but I think it is close enough.
@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Jan 16, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@CAM-Gerlach CAM-Gerlach changed the title PEP 495: Fix incorrect gap image (fixes #2959) PEP 495: Fix incorrect gap image Jan 17, 2023
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

LGTM, thanks, though one minor quibble—could you reduce the density of the dots on the thin dotted bounding lines by at least half, to match the original? It makes them look substantially more prominent, and makes the sketch a little harder to quickly overall.

@bxparks
Copy link
Contributor Author

bxparks commented Jan 17, 2023

I'm using Google Draw as mentioned in #2965 and it provides only this density for "dotted lines". The other line types are variants of "dashed lines". I don't have access to any other draw program, unless you can suggest an alternative that doesn't cost money (I'd rather not spend money just for this bug).

Now, for consistency, I'm willing to manually reproduce the Fold diagram (pep-0495-fold.svg) using the same Google Draw style.

CAM-Gerlach
CAM-Gerlach previously approved these changes Jan 17, 2023
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Oh, right—I'd somehow assumed you were generating it using Matplotlib, Inkscape or by hand (all free alternatives you could consider looking in to in the future), even though you'd told me before you were using Google Draw. My mistake, sorry!

In any case, that's fine—LGTM, thanks.

@CAM-Gerlach CAM-Gerlach dismissed their stale review January 21, 2023 08:23

Seems like the original SVG may be recoverable after all

@CAM-Gerlach
Copy link
Member

FYI, seems like @abalkin may be able to retrieve the original SVG after all, which will match more closely and without the dotted line issue, so let's wait to see about that.

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

Still LGTM and @abalkin confirms the original image isn't easily recoverable, and he's good with merging this 👍 . Thanks @bxparks !

@CAM-Gerlach CAM-Gerlach merged commit 52d0109 into python:main Jan 22, 2023
@Rosuav
Copy link
Contributor

Rosuav commented Jan 22, 2023

Awesome

JelleZijlstra pushed a commit to JelleZijlstra/peps that referenced this pull request Jan 24, 2023
The SVG image in the "Mind the Gap" section (pep-0495-gap.svg) was incorrect.
It is essentially the same as the the SVG image in the "In the Gap" section (pep-0495-fold.svg).
This is a bug that dates back to the original commit on
2015-08-30 (814d372)
that uploaded the both the PNG and SVG versions of the Fold and Gap images.
The PNG version of the Gap image is correct, but the SVG version is incorrect.

The replacement SVG image attached in this PR is a manual recreation
of the PNG version of the Gap image that was deleted on 2022-04-23.
It is not exactly the same, but I think it is close enough.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PEP 495 contains incorrect diagram for Mind the Gap section
3 participants