py3: fix last doctest in cpython folder

[chapoton]

using hash() as suggested in #27886

[chapoton]

New commits:

​b0cdda7

py3: fix last doctests in cpython folder

[chapoton]

an easy one

[vklein]

Looks good to me.

[embray]

Was literally in the process of saying the same just now :)

[embray]

Oops!

[vklein]

Let's be fair.

[jdemeyer]

There is nothing wrong with the Sage code, it should work. It would be better to just apply the upstream fix ​https://github.com/cython/cython/commit/28251032f86c266065e4976080230481b1a1bb29 instead.

[chapoton]

Better maybe, but not for the goal of having all tests pass soon in python3.

Jeroen, will you be able to provide your alternative solution as a branch ? You seem to have less activity on trac, so I guess you have more urgent matters to handle.

Otherwise, maybe this could be done in another ticket ?

[jdemeyer]

Replying to chapoton:

Jeroen, will you be able to provide your alternative solution as a branch ?

Should I add it to #27886? It should take little effort.

[chapoton]

Yes, please. And hopefully someone will review that ticket...

[jdemeyer]

Replying to chapoton:

You seem to have less activity on trac, so I guess you have more urgent matters to handle.

Not really "more urgent matters" but more "other matters".

In case you are wondering what I have been doing recently: ​https://docs.python.org/3.8/whatsnew/3.8.html#vectorcall-a-fast-calling-protocol-for-cpython

[embray]

There is a very simple workaround here, that makes logical sense for the tests (passing an object through hash() necessarily returns whatever the acceptable type for a hash is). Let's just fix this without having to rely yet again on patching cython or whatever.

[jdemeyer]

I already added the patch to #27886.

[jhpalmieri]

I feel like the changes here make sense independently of the Cython patch: I believe that

sage: D = {1230981308409180310928308123: ZZ} # random large integer
sage: test_del_dictitem_by_exact_value(D, ZZ, 1230981308409180310928308123)

will fail, while

sage: D = {1230981308409180310928308123: ZZ}
sage: test_del_dictitem_by_exact_value(D, ZZ, hash(1230981308409180310928308123))

will work.

I don't object to testing whether small integers are equal to their hash values, but that is tested in integer.pyx. After the changes here, the doctest still tests the appropriate function, and they also indicate how to use the function:

sage: D = {'a': ZZ}
sage: test_del_dictitem_by_exact_value(D, ZZ, hash('a'))

[jdemeyer]

Replying to jhpalmieri:

I don't object to testing whether small integers are equal to their hash values, but that is tested in integer.pyx.

True, but unrelated to this ticket. That's not what this doctest is testing.

[jhpalmieri]

Are you agreeing with me or disagreeing? I can't tell.

What is the doctest supposed to be testing?

[jdemeyer]

Replying to jhpalmieri:

What is the doctest supposed to be testing?

That nothing happens if you give the wrong hash value. So basically the test relies on hash(1) != 2.

[jhpalmieri]

The second doctest relies on hash(1) == 1. Well, it doesn't rely on it, but if hash(1) != 1, then I think it is not testing what is intended.

And of course both rely on Sage Integers being valid hash values, which is what is fixed by the patch you posted to #27886.

[embray]

Tickets still needing working or clarification should be moved to the next release milestone at the soonest (please feel free to revert if you think the ticket is close to being resolved).

[chapoton]

should we close this one as invalid ?

[jhpalmieri]

Because of #28112?

[chapoton]

rather because of the cython upgrade in #27886, which fixes these doctests in py3

[vklein]

Replying to chapoton:

should we close this one as invalid ?

Sure. All cpython's module tests pass with 8.9.beta1/py3.