Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

GH-84783: Make the slice object hashable#101264

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
rhettinger merged 15 commits intopython:mainfromfurkanonder:issue-84783
Feb 19, 2023

Conversation

@furkanonder
Copy link
Contributor

@furkanonderfurkanonder commentedJan 23, 2023
edited by bedevere-bot
Loading

@bedevere-bot
Copy link

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 phraseI 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.

Copy link
Contributor

@rhettingerrhettinger left a comment

Choose a reason for hiding this comment

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

We do need to add a NEWS blurb and aversionchanged entry to the docs.

@bedevere-bot
Copy link

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 phraseI 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.

@rhettinger
Copy link
Contributor

We're getting some test failures:

======================================================================[3683](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3684)ERROR: test_sequence_del_slice (test.test_capi.test_misc.CAPITest.test_sequence_del_slice)[3684](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3685)----------------------------------------------------------------------[3685](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3686)Traceback (most recent call last):[3686](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3687)  File "/Users/runner/work/cpython/cpython/Lib/test/test_capi/test_misc.py", line 459, in test_sequence_del_slice[3687](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3688)    _testcapi.sequence_del_slice(mapping, 1, 3)[3688](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3689)KeyError: slice(1, 3, None)[3689](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3690)[3690](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3691)======================================================================[3691](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3692)FAIL: test_sequence_set_slice (test.test_capi.test_misc.CAPITest.test_sequence_set_slice)[3692](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3693)----------------------------------------------------------------------[3693](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3694)Traceback (most recent call last):[3694](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3695)  File "/Users/runner/work/cpython/cpython/Lib/test/test_capi/test_misc.py", line 419, in test_sequence_set_slice[3695](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3696)    with self.assertRaises(TypeError):[3696](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3697)AssertionError: TypeError not raised[3697](https://github.com/python/cpython/actions/runs/4006251040/jobs/6877559173#step:7:3698)

@OTheDev
Copy link
Contributor

It looks like the following test needs to be removed or changed as the expected behaviour would now be aKeyError as slice objects would now be valid as keys.

mapping= {1:'a',2:'b',3:'c'}
withself.assertRaises(TypeError):
_testcapi.sequence_del_slice(mapping,1,3)
self.assertEqual(mapping, {1:'a',2:'b',3:'c'})

The following test can probably be modified to reflect the fact thatmapping[1:3] = 'xy' would be valid and that

mapping== {1:'a',2:'b',3:'c',slice(1,3,None):'xy'}

is true afterwards:

mapping= {1:'a',2:'b',3:'c'}
withself.assertRaises(TypeError):
_testcapi.sequence_set_slice(mapping,1,3,'xy')
self.assertEqual(mapping, {1:'a',2:'b',3:'c'})

furkanonder and rhettinger reacted with thumbs up emoji

@OTheDev
Copy link
Contributor

OTheDev commentedFeb 12, 2023
edited
Loading

Please correct me if I am wrong, but it looks liketest_doctest() is failing because

>>> 825 < len(tests) < 845 # approximate number of objects with docstrings
needs to be relaxed (increased) a bit. It seems to just be at the limit. Non-debug build passes (withlen(tests) at844) but debug build adds one tolen(tests) (making it845). Looking at the commit history, it looks like it needs modifying every now and then.

@iritkatriel Apologies, but it looks like you were the last to modify this line. Does this sound about right to you?

EDIT: I think I've figured it out (below)!

furkanonder reacted with thumbs up emoji

@OTheDev
Copy link
Contributor

OTheDev commentedFeb 12, 2023
edited
Loading

@furkanonder

I think I understand now whytest_doctests() is failing.

On my system, if I clonemain, build it normally (non-debug), and computelen(tests) (see previous comment), I get843.

Now, if I clone your branch, build it normally (non-debug), I get844 instead. The extra entry totests is<DocTest builtins.slice.__hash__ from builtins:None (no examples)>, which isn't an element oftests onmain.

Now, running the tests (on your branch) with non-debug mode succeeds on my system.

Once we build with--with-pydebug, however, this adds an additional entry totests, namely,<DocTest builtins.set.test_c_api from builtins:None (no examples)> (whether on your branch ormain), making the number on your branch845, which makes

>>> 825 < len(tests) < 845 # approximate number of objects with docstrings

fail.

So, I think what needs to be done now is for you to modify that line intest_doctest.py, say, to

>>>830<len(tests)<850

I have verified thattest_doctests() passes after this change (both on non-debug and debug builds).

For reference:

>>>importbuiltins,doctest>>>tests=doctest.DocTestFinder().find(builtins)
furkanonder reacted with thumbs up emoji

OTheDevand others added2 commitsFebruary 12, 2023 21:36
Co-authored-by: Owain Davies <116417456+OTheDev@users.noreply.github.com>
@rhettingerrhettinger merged commit61f1e67 intopython:mainFeb 19, 2023
carljm added a commit to carljm/cpython that referenced this pull requestFeb 20, 2023
* main: (60 commits)pythongh-102056: Fix a few bugs in error handling of exception printing code (python#102078)pythongh-102011: use sys.exception() instead of sys.exc_info() in docs where possible (python#102012)pythongh-101566: Sync with zipp 3.14. (pythonGH-102018)pythonGH-99818: improve the documentation for zipfile.Path and Traversable (pythonGH-101589)pythongh-88233: zipfile: handle extras after a zip64 extra (pythonGH-96161)pythongh-101981: Apply HOMEBREW related environment variables (pythongh-102074)pythongh-101907: Stop using `_Py_OPCODE` and `_Py_OPARG` macros (pythonGH-101912)pythongh-101819: Adapt _io types to heap types, batch 1 (pythonGH-101949)pythongh-101981: Build macOS as recommended by the devguide (pythonGH-102070)pythongh-97786: Fix compiler warnings in pytime.c (python#101826)pythongh-101578: Amend PyErr_{Set,Get}RaisedException docs (python#101962)  Misc improvements to the float tutorial (pythonGH-102052)pythongh-85417: Clarify behaviour on branch cuts in cmath module (python#102046)pythongh-100425: Update tutorial docs related to sum() accuracy (FH-101854)  Add missing 'is' to `cmath.log()` docstring (python#102049)pythongh-100210: Correct the comment link for unescaping HTML (python#100212)pythongh-97930: Also include subdirectory in makefile. (python#102030)pythongh-99735: Use required=True in argparse subparsers example (python#100927)  Fix incorrectly documented attribute in csv docs (python#101250)pythonGH-84783: Make the slice object hashable (pythonGH-101264)  ...
carljm added a commit to carljm/cpython that referenced this pull requestFeb 22, 2023
* main: (225 commits)pythongh-102056: Fix a few bugs in error handling of exception printing code (python#102078)pythongh-102011: use sys.exception() instead of sys.exc_info() in docs where possible (python#102012)pythongh-101566: Sync with zipp 3.14. (pythonGH-102018)pythonGH-99818: improve the documentation for zipfile.Path and Traversable (pythonGH-101589)pythongh-88233: zipfile: handle extras after a zip64 extra (pythonGH-96161)pythongh-101981: Apply HOMEBREW related environment variables (pythongh-102074)pythongh-101907: Stop using `_Py_OPCODE` and `_Py_OPARG` macros (pythonGH-101912)pythongh-101819: Adapt _io types to heap types, batch 1 (pythonGH-101949)pythongh-101981: Build macOS as recommended by the devguide (pythonGH-102070)pythongh-97786: Fix compiler warnings in pytime.c (python#101826)pythongh-101578: Amend PyErr_{Set,Get}RaisedException docs (python#101962)  Misc improvements to the float tutorial (pythonGH-102052)pythongh-85417: Clarify behaviour on branch cuts in cmath module (python#102046)pythongh-100425: Update tutorial docs related to sum() accuracy (FH-101854)  Add missing 'is' to `cmath.log()` docstring (python#102049)pythongh-100210: Correct the comment link for unescaping HTML (python#100212)pythongh-97930: Also include subdirectory in makefile. (python#102030)pythongh-99735: Use required=True in argparse subparsers example (python#100927)  Fix incorrectly documented attribute in csv docs (python#101250)pythonGH-84783: Make the slice object hashable (pythonGH-101264)  ...
Yhg1s added a commit to Yhg1s/cpython that referenced this pull requestMar 9, 2023
furkanonder added a commit to furkanonder/cpython that referenced this pull requestMar 31, 2023
terryjreedy added a commit that referenced this pull requestMar 31, 2023
Will Bradshaw contributed original patch onbpo-40603.---------Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
warsaw pushed a commit to warsaw/cpython that referenced this pull requestApr 11, 2023
…ble) (python#103146)Will Bradshaw contributed original patch on bpo-40603.---------Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@rhettingerrhettingerrhettinger requested changes

+1 more reviewer

@OTheDevOTheDevOTheDev left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@furkanonder@bedevere-bot@rhettinger@OTheDev

[8]ページ先頭

©2009-2025 Movatter.jp