[with patch, with positive review] RDF/CDF coverage, consistent hashing

[Robert Bradshaw]

Some functions, e.g. one-liners and stuff dealing with the RDF pool, are hard or useless to test in the doctest setting.

Hashing is now consistent between RDF, RR, CDF, CC, float, and complex.

[Mike Hansen]

I get the following error after the patch:

    sage: whole, parts = q.partial_fraction_decomposition(); parts
Expected:
    [(-7.6294e-6*x^2 + 1.0000)/(1.0000*x^4 + 4.0000*x^2 + 4.0000), 1.0000/(1.0000*x - 1.0000)]
Got:
    [1.0000/(1.0000*x - 1.0000), (-7.6294e-6*x^2 + 1.0000)/(1.0000*x^4 + 4.0000*x^2 + 4.0000)]

[Robert Bradshaw]

This is just an ordering issue (which is arbitrary), the doctest can be changed.

[ncalexan]

Patch looks excellent. I would like the int(...) and long(...) functions to actually doctest that their output has the expected type, but that's a small issue.

I cannot speak to whether the hashing strategy is a good design decision.

[ncalexan]

robertwb claims "Hashing is now consistent between RDF, RR, CDF, CC, float, and complex."

Is this doctested? I don't think so. It should be. Should this maybe go in the module level tests for RDF, CDF, or something like that?

[Robert Bradshaw]

Yes, the consistency between RDF, RR, CDF, CC, float, and complex is doctested, the doctests for the __hash__ functions each test against hashing the native Python type.

[Carl Witty]

Once both rdf-cdf-coverage.diff and rdf-cdf-fixup.patch are applied, all doctests pass on both 32-bit and 64-bit x86 Linux.

The code looks good.

[Michael Abshoff]

I had a trivial reject for Robert's patch which I resolved manually. See

​http://sage.math.washington.edu/home/mabshoff/release-cycles-2.10.1/rc2/trac_1823_rdf-cdf-coverage-missing-hunk.patch

Cheers,

Michael

[Michael Abshoff]

Merged in Sage 2.10.1.rc2

[Michael Abshoff]

[23:19] <mabshoff> cwitty: after applying #1823 I have a comple_double.h and complex_double_api.h 
[23:20] <mabshoff> in sage/rings all the sudden.
[23:20] <mabshoff> And hg claims they are not under revision control. Any idea what is up with that?
[23:22] <cwitty> mabshoff, strange Cython magic?
[23:23] <mabshoff> No clue. I grepped the tree for any file including them and there are none.
[23:23] <cwitty> They're clearly autogenerated, and are probably caused by this:
[23:23] <cwitty> cdef public api ComplexDoubleElement new_ComplexDoubleElement()
[23:23] <cwitty> from Robert's patch.
[23:23] <mabshoff> ok
[23:23] <cwitty> But I don't know why Robert thought that should be public.
[23:24] <mabshoff> Maybe something that he will follow up with?
[23:24] <mabshoff> But what to do? Put them into the repo? I don't think we will get an answer today.
[23:25] <cwitty> No; I think add them to .hgignore.
[23:25] <cwitty> (Auto-generated files shouldn't go in the repository.)

Ad per cwitty suggestion I added the to .hgignore. The patch attached above has one extra, erroneous change (sage->SAGE [damn vi]), that I reverted already in my repo. It is just attached for the record. If adding those two files to .hgignore was a mistake please let us know.

Cheers,

Michael