GH-84783: Make the slice object hashable #101264

furkanonder · 2023-01-23T20:08:46Z

Issue: slice not hashable #84783

Objects/sliceobject.c

bedevere-bot · 2023-02-05T21:17:45Z

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.

rhettinger

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

bedevere-bot · 2023-02-05T21:42:09Z

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.

rhettinger · 2023-02-05T21:44:02Z

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)

rhettinger · 2023-02-05T21:45:03Z

Objects/sliceobject.c

+slicehash(PySliceObject *v)
+{
+    Py_uhash_t acc = _PyHASH_XXPRIME_5;
+#define _PyHASH_SLICE_PART(com) { \


It would look nicer if this macro were replaced with a static inline function.

OTheDev · 2023-02-11T14:32:48Z

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

cpython/Lib/test/test_capi/test_misc.py

Lines 457 to 460 in b652d40

    
           mapping = {1: 'a', 2: 'b', 3: 'c'} 
        
           with self.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 that mapping[1:3] = 'xy' would be valid and that

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

is true afterwards:

cpython/Lib/test/test_capi/test_misc.py

Lines 418 to 421 in b652d40

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

Lib/test/test_slice.py

Doc/library/functions.rst

OTheDev · 2023-02-12T03:57:11Z

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

cpython/Lib/test/test_doctest.py

Line 710 in 3eb12df

>>> 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 (with len(tests) at 844) but debug build adds one to len(tests) (making it 845). 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)!

OTheDev · 2023-02-12T11:36:06Z

@furkanonder

I think I understand now why test_doctests() is failing.

On my system, if I clone main, build it normally (non-debug), and compute len(tests) (see previous comment), I get 843.

Now, if I clone your branch, build it normally (non-debug), I get 844 instead. The extra entry to tests is <DocTest builtins.slice.__hash__ from builtins:None (no examples)>, which isn't an element of tests on main.

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 to tests, namely, <DocTest builtins.set.test_c_api from builtins:None (no examples)> (whether on your branch or main), making the number on your branch 845, which makes

cpython/Lib/test/test_doctest.py

Line 710 in 3eb12df

>>> 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 in test_doctest.py, say, to

>>> 830 < len(tests) < 850

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

For reference:

>>> import builtins, doctest
>>> tests = doctest.DocTestFinder().find(builtins)

Co-authored-by: Owain Davies <116417456+OTheDev@users.noreply.github.com>

Update test_doctest.py

Lib/test/test_slice.py

OTheDev · 2023-02-13T21:06:05Z

Lib/test/test_slice.py

+        self.assertEqual(hash(slice(5)), slice(5).__hash__())
+        self.assertEqual(hash(slice(1, 2)), slice(1, 2).__hash__())
+        self.assertEqual(hash(slice(1, 2, 3)), slice(1, 2, 3).__hash__())
+        self.assertNotEqual(slice(5), slice(6))


Why test this (line 86)? Isn't that what test_cmp() is for?

* 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) ...

* 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) ...

…'s New. (#102548)

Will Bradshaw contributed original patch on bpo-40603. --------- Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

…ble) (python#103146) Will Bradshaw contributed original patch on bpo-40603. --------- Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

furkanonder added 2 commits January 23, 2023 23:03

fix the typo in the tuplehash func's comment

61c8048

make hashable the slice

ef22a92

bedevere-bot added the awaiting review label Jan 23, 2023

bedevere-bot mentioned this pull request Jan 23, 2023

slice not hashable #84783

Closed

furkanonder added 2 commits January 23, 2023 20:09

Merge branch 'main' into issue-84783

12c2488

Merge branch 'main' into issue-84783

6dd9926

rhettinger requested changes Feb 5, 2023

View reviewed changes

Objects/sliceobject.c Show resolved Hide resolved

bedevere-bot removed the awaiting review label Feb 5, 2023

bedevere-bot added the awaiting changes label Feb 5, 2023

rhettinger approved these changes Feb 5, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting changes labels Feb 5, 2023

rhettinger requested changes Feb 5, 2023

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting merge labels Feb 5, 2023

rhettinger reviewed Feb 5, 2023

View reviewed changes

furkanonder and others added 3 commits February 12, 2023 01:45

fix failed slice's test cases

886bfff

Merge branch 'main' into issue-84783

2971147

📜🤖 Added by blurb_it.

093bf38

OTheDev reviewed Feb 11, 2023

View reviewed changes

Lib/test/test_slice.py Show resolved Hide resolved

Add version changed to slice's doc

159f3f2

OTheDev reviewed Feb 12, 2023

View reviewed changes

Doc/library/functions.rst Outdated Show resolved Hide resolved

OTheDev and others added 2 commits February 12, 2023 21:36

Update test_doctest.py

0c78cfc

Update Doc/library/functions.rst

cff08dd

Co-authored-by: Owain Davies <116417456+OTheDev@users.noreply.github.com>

furkanonder added 4 commits February 12, 2023 20:42

Merge branch 'main' into issue-84783

ea55931

Update version changed to slice's doc

f0302bf

Merge pull request #1 from OTheDev/patch-1

2c76491

Update test_doctest.py

Add more testcases for slice's hash

cbd1527

furkanonder requested a review from OTheDev February 12, 2023 23:02

OTheDev reviewed Feb 13, 2023

View reviewed changes

Lib/test/test_slice.py Outdated Show resolved Hide resolved

update testcases for slice's hash

9f55e46

OTheDev reviewed Feb 13, 2023

View reviewed changes

rhettinger merged commit 61f1e67 into python:main Feb 19, 2023

bedevere-bot removed the awaiting changes label Feb 19, 2023

Yhg1s added a commit to Yhg1s/cpython that referenced this pull request Mar 9, 2023

Document pythonGH-101264 (Make the slice object hashable) in What's New.

7c4a026

Yhg1s added a commit that referenced this pull request Mar 9, 2023

GH-84783: Document GH-101264 (Make the slice object hashable) in What…

58d761e

…'s New. (#102548)

furkanonder added a commit to furkanonder/cpython that referenced this pull request Mar 31, 2023

Mention Author for pythonGH-101264 (Make the slice object hashable)

b1edad1

terryjreedy added a commit that referenced this pull request Mar 31, 2023

GH-84783: Mention Author for GH-101264 (make slices hashable) (#103146)

048d624

Will Bradshaw contributed original patch on bpo-40603. --------- Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

zzzeek mentioned this pull request May 22, 2023

python 3.12 slices are hashable, affects one area of Row for 1.4 only sqlalchemy/sqlalchemy#9819

Closed

lithomas1 mentioned this pull request Jul 11, 2023

CI: Test Python 3.12 pandas-dev/pandas#53743

Merged

5 tasks

sondreso mentioned this pull request Nov 24, 2023

Build python 3.12 wheels in Github Actions equinor/segyio#563

Merged

tehunter mentioned this pull request Mar 27, 2024

BUG: Columns resulting from joining multiIndex dataFrame are incorrect in python 3.12.1 pandas-dev/pandas#57500

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-84783: Make the slice object hashable #101264

GH-84783: Make the slice object hashable #101264

furkanonder commented Jan 23, 2023 •

edited by bedevere-bot

Loading

bedevere-bot commented Feb 5, 2023

rhettinger left a comment

bedevere-bot commented Feb 5, 2023

rhettinger commented Feb 5, 2023

rhettinger Feb 5, 2023

OTheDev commented Feb 11, 2023

OTheDev commented Feb 12, 2023 •

edited

Loading

OTheDev commented Feb 12, 2023 •

edited

Loading

OTheDev Feb 13, 2023 •

edited

Loading

GH-84783: Make the slice object hashable #101264

GH-84783: Make the slice object hashable #101264

Conversation

furkanonder commented Jan 23, 2023 • edited by bedevere-bot Loading

bedevere-bot commented Feb 5, 2023

rhettinger left a comment

Choose a reason for hiding this comment

bedevere-bot commented Feb 5, 2023

rhettinger commented Feb 5, 2023

rhettinger Feb 5, 2023

Choose a reason for hiding this comment

OTheDev commented Feb 11, 2023

OTheDev commented Feb 12, 2023 • edited Loading

OTheDev commented Feb 12, 2023 • edited Loading

OTheDev Feb 13, 2023 • edited Loading

Choose a reason for hiding this comment

furkanonder commented Jan 23, 2023 •

edited by bedevere-bot

Loading

OTheDev commented Feb 12, 2023 •

edited

Loading

OTheDev commented Feb 12, 2023 •

edited

Loading

OTheDev Feb 13, 2023 •

edited

Loading