Opened 6 years ago
Closed 6 years ago
#16992 closed enhancement (fixed)
Direct Fixdoctests
Reported by: | vbraun | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.4 |
Component: | scripts | Keywords: | |
Cc: | Merged in: | ||
Authors: | Volker Braun | Reviewers: | Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | 1b99c68 (Commits, GitHub, GitLab) | Commit: | 1b99c68fdfe02cbbbc57fe4dab882505a136d3e6 |
Dependencies: | Stopgaps: |
Description (last modified by )
Now that we are actually using a versions control system as it is intended, the sage --fixdoctests
workflow is a bit antiquated:
- Run
sage --fixdoctests src/sage/foo.py
- Get a diff printed to stdout
- copy
src/sage/foo.py.out
tosrc/sage/foo.py
- Run
git diff
to get the same diff again
Instead, just overwrite the file directly. Use version control for the rest.
Change History (17)
comment:1 Changed 6 years ago by
- Branch set to u/vbraun/direct_fixdoctests
comment:2 Changed 6 years ago by
- Commit set to 8c5f2b0e915d525b7984b43721a869b6040ed20f
- Component changed from PLEASE CHANGE to scripts
- Description modified (diff)
- Status changed from new to needs_review
- Type changed from PLEASE CHANGE to enhancement
comment:3 Changed 6 years ago by
I would replace sage -git
by git
since this is supposed to be run from a Sage shell anyway.
comment:4 Changed 6 years ago by
- Commit changed from 8c5f2b0e915d525b7984b43721a869b6040ed20f to da59f20583c9a9aeb0ca64c9a1403da412666bd6
Branch pushed to git repo; I updated commit sha1. New commits:
da59f20 | call git directly and from the right directory
|
comment:5 Changed 6 years ago by
- Status changed from needs_review to needs_work
You also need to fix the documentation (output of ./sage --advanced
)
comment:6 Changed 6 years ago by
And could you please add a \n
to the output file, because otherwise you always get
-
src/sage/rings/integer.pyx
diff --git a/src/sage/rings/integer.pyx b/src/sage/rings/integer.pyx index c85ba09..59cb6c1 100644
a b cdef double mpz_get_d_nearest(mpz_t x) except? -648555075988944.5: 6515 6515 cdef double d = <double>q64 6516 6516 if resultsign < 0: 6517 6517 d = -d 6518 return ldexp(d, shift) 6518 return ldexp(d, shift) 6519 No newline at end of file
(I know you didn't introduce this bug, but it's such a trivial fix...)
comment:7 Changed 6 years ago by
Shouldn't
if relative.startswith('.'):
be
if relative.startswith('..'):
(although it seems that relpath
strips ./
)
comment:8 Changed 6 years ago by
- Reviewers set to Jeroen Demeyer
There is of course a large margin for improvement in the sage-fixdoctests
script (ideally, it should be integrated within the doctesting framework), but if you make the small changes I mentioned, I'm happy with this ticket.
comment:10 Changed 6 years ago by
- Commit changed from da59f20583c9a9aeb0ca64c9a1403da412666bd6 to e397b3e6b1dfa0c0a7877adb89efa278495000f9
- Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
e397b3e | Add newline at end of file and fix cmdline help
|
comment:11 Changed 6 years ago by
- Status changed from needs_review to positive_review
comment:12 Changed 6 years ago by
- Commit changed from e397b3e6b1dfa0c0a7877adb89efa278495000f9 to 1b99c68fdfe02cbbbc57fe4dab882505a136d3e6
- Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
1b99c68 | Also fix the cmdline doctest
|
comment:13 Changed 6 years ago by
- Status changed from needs_review to positive_review
Forgot the cmdline test...
comment:14 Changed 6 years ago by
Any reason you write
with open(test_file, 'r') as f: fixed_test = f.read()
instead of
fixed_test = open(test_file).read()
comment:15 Changed 6 years ago by
I don't really care either way much, but to me the first version makes it clearer how long the file is actually open for.
comment:16 Changed 6 years ago by
lgtm (I was going to set positive_review but apparently you already did that :-p)
comment:17 Changed 6 years ago by
- Branch changed from u/vbraun/direct_fixdoctests to 1b99c68fdfe02cbbbc57fe4dab882505a136d3e6
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Directly overwrite with fixed doctests