#16889 closed defect (fixed)
Doctest tolerance: allow spaces and use intervals
Reported by:  jdemeyer  Owned by:  

Priority:  blocker  Milestone:  sage6.4 
Component:  doctest framework  Keywords:  
Cc:  zimmerma  Merged in:  sage6.4.beta3 
Authors:  Jeroen Demeyer  Reviewers:  Volker Braun 
Report Upstream:  N/A  Work issues:  
Branch:  1f858c7 (Commits, GitHub, GitLab)  Commit:  1f858c7511d147e2f93dd0fd4e160fed8ad4f234 
Dependencies:  Stopgaps: 
Description (last modified by )
The doctesting framework should ignore whitespace before numbers and between the sign and the number when testing with (absrel) tol
. There are two reasons why:
 The spacing of matrices is unpredictable. You could have
[1.0 0.0] [1.0 0.0]
vs
[0.999999 0.0] [ 1.0 0.0]
Note the added space between [
and 1.0
.
 Complex numbers or polynomials
1.0 + 1e16*I
vs
1.0  1e16*I
Here you want to compare + 1e16
and  1e16
with the sign for your tolerance computations.
After some consideration, I decided it actually makes a lot of sense to use interval arithmetic for doctest tolerances, this is implemented on this ticket.
Change History (21)
comment:1 Changed 7 years ago by
 Branch set to u/jdemeyer/ticket/16889
 Created changed from 08/27/14 14:43:50 to 08/27/14 14:43:50
 Modified changed from 08/27/14 14:43:50 to 08/27/14 14:43:50
comment:2 Changed 7 years ago by
 Commit set to 46301acf040394a5660c7a9a7262116cfc721804
comment:3 Changed 7 years ago by
 Commit changed from 46301acf040394a5660c7a9a7262116cfc721804 to 291a442c8b006a8224e76cc68df7ad9708333c96
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
291a442  Doctest tolerance: allow spaces

comment:4 Changed 7 years ago by
 Cc zimmerma added
Paul Zimmermann: this needs a minor change to a French book doctest. When using # tol
, you may no longer split doctest results like
1.5  2.3
The minus sign may not be on its own line, you need to write
1.5  2.3
comment:5 Changed 7 years ago by
 Description modified (diff)
 Status changed from new to needs_review
comment:6 followup: ↓ 7 Changed 7 years ago by
in the source of the french book, I've changed to:
[0.347521510119 + 0.566550553398*I, 0.347521510119  0.566550553398*I, 0.345023776962 + 0.439908702386*I, 0.345023776962  0.439908702386*I, 0.517257614325 + 0.512958206789*I, 0.517257614325  0.512958206789*I, 1.36699716455, 9.98357818097]
which uses the same number of lines, and should not make any overfull hbox. Is that ok?
Paul
comment:7 in reply to: ↑ 6 Changed 7 years ago by
Replying to zimmerma:
in the source of the french book, I've changed to:
[0.347521510119 + 0.566550553398*I, 0.347521510119  0.566550553398*I, 0.345023776962 + 0.439908702386*I, 0.345023776962  0.439908702386*I, 0.517257614325 + 0.512958206789*I, 0.517257614325  0.512958206789*I, 1.36699716455, 9.98357818097]which uses the same number of lines, and should not make any overfull hbox. Is that ok?
Yes, that would be good. However, note that Sage itself actually formats the results as in my patch, with one value per line.
comment:8 Changed 7 years ago by
ok, however in our book we allow ourselves not to follow 100% the Sage output format, up to spaces and line breaks, to save some vertical space.
Paul
comment:9 Changed 7 years ago by
Fun fact: this ticket relies on functionality added in 2006:

src/sage/ext/mpfr.pyx
commit 04043acc648e0bca14725f147b2040e204ae1f9b Author: William Stein <wstein@ucsd.edu> Date: Mon May 29 01:10:56 2006 +0000 [project @ mpfr  can coerce in numbers ignoring space, e.g. RR('1 E7') now works (which is what pari outputs!)] diff git a/src/sage/ext/mpfr.pyx b/src/sage/ext/mpfr.pyx index 8f92208..afc9b72 100644
a b cdef class RealNumber(element.RingElement): 602 602 d = parent(x.denominator()) 603 603 mpfr_div(self.value, n.value, d.value, parent.rnd) 604 604 else: 605 s = str(x) 605 s = str(x).replace(' ','') 606 606 if mpfr_set_str(self.value, s, base, parent.rnd): 607 607 if s == 'NaN' or s == '@NaN@': 608 608 mpfr_set_nan(self.value)
comment:10 Changed 7 years ago by
 Description modified (diff)
comment:11 Changed 7 years ago by
 Commit changed from 291a442c8b006a8224e76cc68df7ad9708333c96 to dde3a3a13ad9d78afe014743d31b766e0ced2af8
Branch pushed to git repo; I updated commit sha1. New commits:
dde3a3a  Use intervals for doctest tolerances

comment:12 Changed 7 years ago by
 Description modified (diff)
 Summary changed from Doctest tolerance: allow spaces to Doctest tolerance: allow spaces and use intervals
comment:13 Changed 7 years ago by
 Commit changed from dde3a3a13ad9d78afe014743d31b766e0ced2af8 to 59b511ea9efef56e1e9226216ff67212575e4cde
Branch pushed to git repo; I updated commit sha1. New commits:
59b511e  Add tolerance test file

comment:14 Changed 7 years ago by
This looks good to me. How about renaming the tolerance.rst
to must_fail.rst
or so and have an explanatory sentence about what is going on at the top. I fear that somebody will sooner or later "fix" the failing doctests if its not clear ;)
comment:15 Changed 7 years ago by
 Commit changed from 59b511ea9efef56e1e9226216ff67212575e4cde to 1f858c7511d147e2f93dd0fd4e160fed8ad4f234
Branch pushed to git repo; I updated commit sha1. New commits:
1f858c7  Add comment that we test failing tests

comment:16 Changed 7 years ago by
 Reviewers set to Volker Braun
 Status changed from needs_review to positive_review
comment:17 Changed 7 years ago by
 Status changed from positive_review to needs_work
Doctests fail because the line numbers are shifted, e.g. http://build.sagemath.org/sage/builders/%20%20fast%20Volker%20Desktop%20%28Fedora%2020%20x86_64%29%20incremental/builds/416/steps/shell_4/logs/stdio
comment:18 Changed 7 years ago by
 Branch changed from u/jdemeyer/ticket/16889 to u/vbraun/ticket/16889
comment:19 Changed 7 years ago by
 Modified changed from 09/01/14 21:44:18 to 09/01/14 21:44:18
 Status changed from needs_work to positive_review
comment:20 Changed 7 years ago by
 Branch changed from u/vbraun/ticket/16889 to 1f858c7511d147e2f93dd0fd4e160fed8ad4f234
 Resolution set to fixed
 Status changed from positive_review to closed
comment:21 Changed 6 years ago by
 Merged in set to sage6.4.beta3
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
Doctest tolerance: allow spaces