GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-37004: Documented asymmetry of string arguments in difflib.SequenceMatcher for ratio method by sweeneyde · Pull Request #13482 · python/cpython

Conversation

@sweeneyde

@sweeneyde sweeneyde commented May 22, 2019 •
edited by miss-islington
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

https://bugs.python.org/issue37004

Automerge-Triggered-By: @tim-one

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@sweeneyde
Warning that SequenceMatcher.ratio() is not commutative
a590a97
@the-knights-who-say-ni the-knights-who-say-ni added the CLA not signed label May 22, 2019
@the-knights-who-say-ni

the-knights-who-say-ni commented May 22, 2019

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels May 22, 2019
tim-one
tim-one suggested changes May 22, 2019
View reviewed changes

@tim-one tim-one left a comment

Copy link
Copy Markdown
Member

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 like the idea! But want some changes, as spelled out in comments. Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Doc/library/difflib.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/difflib.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot added awaiting changes and removed awaiting review labels May 22, 2019
@bedevere-bot

bedevere-bot commented May 22, 2019

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sweeneyde
Rephrased SequenceMatcher warning and changed example
a6ab363
@sweeneyde

sweeneyde commented May 22, 2019

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot added awaiting change review and removed awaiting changes labels May 22, 2019
@bedevere-bot

bedevere-bot commented May 22, 2019

Copy link
Copy Markdown

Thanks for making the requested changes!

@tim-one: please review the changes made to this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tim-one
tim-one approved these changes May 22, 2019
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting change review labels May 22, 2019
@sweeneyde
removed trailing whitespace
8f2aab8
tirkarthi
tirkarthi reviewed May 22, 2019
View reviewed changes
Comment thread Doc/library/difflib.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@blurb-it
📜🤖 Added by blurb_it.
0683a5f
@the-knights-who-say-ni the-knights-who-say-ni added CLA signed and removed CLA not signed labels May 22, 2019
@sweeneyde

sweeneyde commented Aug 7, 2019

Copy link
Copy Markdown
Member Author

What is the process for getting this PR merged?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tim-one

tim-one commented Aug 7, 2019

Copy link
Copy Markdown
Member

Thanks for speaking up! While the change is fine by me, I'm not familiar enough with current workflow to finish it. So I just posted a message to the python-dev mailing list, asking for someone else to proceed:

https://mail.python.org/archives/list/[email protected]/thread/TFTQCJ7IZNCE3XUWEC4IVHPXBDZIKVEI/

I bet they'll ask that you add your name to the Misc/ACKS file.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tim-one tim-one added 🤖 automerge labels Aug 7, 2019
@miss-islington

miss-islington commented Aug 7, 2019

Copy link
Copy Markdown
Contributor

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington
miss-islington merged commit e9cbcd0 into python:master Aug 7, 2019
@miss-islington

miss-islington commented Aug 7, 2019

Copy link
Copy Markdown
Contributor

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

1 similar comment
@miss-islington

miss-islington commented Aug 7, 2019

Copy link
Copy Markdown
Contributor

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington

miss-islington commented Aug 7, 2019

Copy link
Copy Markdown
Contributor

Thanks @sweeneyde for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7, 3.8.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the awaiting merge label Aug 7, 2019
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 7, 2019
@sweeneyde @miss-islington
bpo-37004: Documented asymmetry of string arguments in difflib.Sequen…
5fbefa5
…ceMatcher for ratio method (pythonGH-13482)

https://bugs.python.org/issue37004
(cherry picked from commit e9cbcd0)

Co-authored-by: sweeneyde <[email protected]>
@bedevere-bot

bedevere-bot commented Aug 7, 2019

Copy link
Copy Markdown

GH-15157 is a backport of this pull request to the 3.8 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the needs backport to 3.8 label Aug 7, 2019
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 7, 2019
@sweeneyde @miss-islington
bpo-37004: Documented asymmetry of string arguments in difflib.Sequen…
58b0c26
…ceMatcher for ratio method (pythonGH-13482)

https://bugs.python.org/issue37004
(cherry picked from commit e9cbcd0)

Co-authored-by: sweeneyde <[email protected]>
@bedevere-bot

bedevere-bot commented Aug 7, 2019

Copy link
Copy Markdown

GH-15158 is a backport of this pull request to the 3.7 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the needs backport to 3.7 label Aug 7, 2019
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 7, 2019
@sweeneyde @miss-islington
bpo-37004: Documented asymmetry of string arguments in difflib.Sequen…
f7c61a5
…ceMatcher for ratio method (pythonGH-13482)

https://bugs.python.org/issue37004
(cherry picked from commit e9cbcd0)

Co-authored-by: sweeneyde <[email protected]>
@bedevere-bot bedevere-bot removed the needs backport to 3.6 label Aug 7, 2019
@bedevere-bot

bedevere-bot commented Aug 7, 2019

Copy link
Copy Markdown

GH-15159 is a backport of this pull request to the 3.6 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

terryjreedy pushed a commit that referenced this pull request Aug 7, 2019
@miss-islington @sweeneyde @terryjreedy
bpo-37004: Documented asymmetry of string arguments in difflib.Sequen…
1a3a40c
…ceMatcher for ratio method (GH-13482) (#15157)

https://bugs.python.org/issue37004
(cherry picked from commit e9cbcd0)

Co-authored-by: sweeneyde <[email protected]>
terryjreedy pushed a commit that referenced this pull request Aug 7, 2019
@miss-islington @sweeneyde @terryjreedy
bpo-37004: Documented asymmetry of string arguments in difflib.Sequen…
7dafbe8
…ceMatcher for ratio method (GH-13482) (#15158)

https://bugs.python.org/issue37004
(cherry picked from commit e9cbcd0)

Co-authored-by: sweeneyde <[email protected]>
@terryjreedy

terryjreedy commented Aug 7, 2019

Copy link
Copy Markdown
Member

@tim-one The backports have to be 1. approved by a coredev, after briefly checking that done correctly, in which case the bot will merge if and when CI passes, or 2. directly merged by human, after CI passes. I did the latter for 3.8 and 3.7.

tim-one reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tim-one

tim-one commented Aug 7, 2019

Copy link
Copy Markdown
Member

Thanks, Terry! I was just going to kill them, to avoid screwing things up worse ☹️

While you're here, do you understand what the earlier

Sorry, I can't merge this PR. Reason: Base branch was modified. Review and try the merge again..

messages were trying to communicate?

And, when you did the merges, do you understand what the

Please replace # with GH- in the commit message next time. Thanks!

messages were trying to say? I've seen the latter message every time I've "squashed & merged" anything, and still have no idea what it's talking about (I didn't use "#" in the commit messages).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@terryjreedy

terryjreedy commented Aug 7, 2019

Copy link
Copy Markdown
Member
  1. Good thing you left them alone. They were fine. If closed, new backport labels here would have been needed. I open this and took a look at them because your fairly detailed explanation of what you did omitted mention of approving the backports.

  2. Once CI for a patch to master passes, changes to master can create merge conflicts. When so, Github adds a temporary message to the CI detail box. (It is now shrunken, with a [View Details] button.) One can resolved them by editing files with conflict markers either with Github's online editor or with any editor on one's working machine, followed by a push of the edited branch.

When you added the Automerge label, our backport/merge bot, Miss Islington gets involved. I resume its message paraphrases what I said above. But I am puzzled because there is no listed merge-resolution commit. Perhaps there was a spurious conflict that disappeared, perhaps github retried with a different and more successful merge strategy, perhaps there was just a timing issue. Automerge is fairly recent, and I don't use it for changes to master because it hides the human who made the decision, and I often decide to merge after a final review.

  1. The title at the top of this page has the PR number, here bpo-37004: Documented asymmetry of string arguments in difflib.SequenceMatcher for ratio method #13482, at the end, in gray. It cannot be edited. When one presses the green [Merge] button, github adds the number in ()s, such as (bpo-37004: Documented asymmetry of string arguments in difflib.SequenceMatcher for ratio method #13482) in black, and editable. We are supposed to remember to change '#' to 'GH-', because bpo-37004: Documented asymmetry of string arguments in difflib.SequenceMatcher for ratio method #13482 could also refer to a tracker (bpo) number. Automerge does that. (What I don't like about automerge for original PRs is that it hides who triggered the merge.)

The same change is requested for backport numbers on backport merges (although there is discussion that they should maybe be deleted instead, or maybe the original PR #). I forgot.

A coredev approving a backport is equivalent to an automerge label. Then the change is automatic. I could have used that instead. I have not checked yet whether the label is available.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tim-one

tim-one commented Aug 7, 2019

Copy link
Copy Markdown
Member

Thanks again, Terry. Ya, I assumed the "can't merge this PR" messages were complaining about a merge conflict, but didn't see any evidence of such. Plus there were three such messages, two of which followed the bot saying it merged the commit into master.

So then I thought "OK, 3 complaints, but nothing fishy about master, but I added 3 backport labels so it must be complaining about them". But no sign of merge conflicts in those either.

For the rest, sounds like I'll avoid automerge in the future. It's hard enough trying to guess the consequences of what I explicitly do 😉.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
@sweeneyde @lisroach
bpo-37004: Documented asymmetry of string arguments in difflib.Sequen…
cc6ea49
…ceMatcher for ratio method (pythonGH-13482)

https://bugs.python.org/issue37004
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
@sweeneyde @DinoV
bpo-37004: Documented asymmetry of string arguments in difflib.Sequen…
bb33333
…ceMatcher for ratio method (pythonGH-13482)

https://bugs.python.org/issue37004
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
@sweeneyde @websurfer5
bpo-37004: Documented asymmetry of string arguments in difflib.Sequen…
211d812
…ceMatcher for ratio method (pythonGH-13482)

https://bugs.python.org/issue37004
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tirkarthi tirkarthi tirkarthi left review comments
+1 more reviewer
@tim-one tim-one tim-one approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

docs Documentation in the Doc dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@sweeneyde @the-knights-who-say-ni @bedevere-bot @tim-one @miss-islington @terryjreedy @tirkarthi
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.