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 thePSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username onbugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow these steps to help rectify the issue:

1. If you don't have an account on b.p.o, pleasecreate one
2. Make sure your GitHub username is listed in"Your Details" at b.p.o
3. If you have not already done so, please sign thePSF contributor agreement
4. If you just signed the CLA, pleasewait at least one US business day and then check "Your Details" onbugs.python.org to see if your account has been marked as having signed the CLA (the delay is due to a person having to manually check your signed CLA)
5. Reply here saying you have completed the above steps

Thanks again to your contribution and we look forward to looking at it!

Should the commit rebase into one commit?

@grapherd The PR can be accepted as a squash merge in one click that will have the same effect as manually squashing it.

Rebased to HEAD and squashed

I think this looks good now.@serhiy-storchaka would you like to take another look? I can do the backports.
Thanks :)

No backports please.

This is just the documentation fix. I think it should be backported.

But proposed wording doesn't LGTM. See my comment on the tracker.

Onbpo serhiy-storchaka wrote:

Proposed wording doesn't looks much informative to me. Maybe just say that PyMapping_Check() also returns 1 for sequences that support slicing? And recommendPyMapping_Check() && !PySequence_Check() for true mapping test?

There is no recommendation to usePyMapping_Check() && !PySequence_Check() in that thread. In fact developersrecommend against both PyMapping_Check and PySequence_Check as well:

(Author: Benjamin Peterson) Personally, I think PyMapping_Check and PySequence_Check should be deprecated and removed. Like their Python counterparts, operator.isMappingType() and operation.isSequenceType(), they are unreliable at best in the face of not LBYL and abcs

So I think it is appropriate to keep the proposed wording since it steers users to ABC checks and away from these APIs. If there is a change perhaps it should be explicitly "not recommended" in the documentation.

To try and help move older pull requests forward, we are going through and backfilling 'awaiting' labels on pull requests that are lacking the label. Based on the current reviews, the best we can tell in an automated fashion is that a core developer requested changes to be made to this pull request.

If/when the requested changes have been made, please leave a comment that says,I have made the requested changes; please review again. That will trigger a bot to flag this pull request as ready for a follow-up review.

I have made the requested changes; please review again.

Given that several core developers suggested or concurred with deprecating this API (Benjamin Peterson, GvR, Raymond Hettinger) I marked it not recommended as an alternative to officially deprecating.

I think it is best to omit the suggestion to usePyMapping_Check() && !PySequence_Check(). It appears this test does not work in certain cases, becausePyMapping_Check is actually testing an CPython implementation detail. Instead I recommendedPyObject_IsInstance which steers the developer toward the modern ABC API.

Thanks for making the requested changes!

@serhiy-storchaka: please review the changes made to this pull request.

@willingc Done

Thanks@jbarlow83 for the quick response.

@serhiy-storchaka This looks good to me now. If you have no further changes, I recommend merging. Thanks.

Superseded by#7029