Re: [sage-devel] Please review global docstring/test change #16746
On Tuesday, September 23, 2014 7:47:39 PM UTC+1, Jeroen Demeyer wrote: I really would like somebody else (preferably somebody who knows about IPython and displayhooks) to review that part of the ticket. Unless you have somebody specific in mind that is not a very useful position. The goal of review is not to wait until somebody comes along and writes the perfect patch. The question that you should ask yourself is: Is it better than what we currently have. -- You received this message because you are subscribed to the Google Groups sage-devel group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at http://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] Please review global docstring/test change #16746
bump On Thursday, September 18, 2014 9:27:18 PM UTC+1, Jeroen Demeyer wrote: On 2014-09-18 20:30, Volker Braun wrote: I would like to merge the ticket asap and then release the next beta. So please review ;-) I'm not convinced that you should postpone the next beta for this. If you think you have a candidate next beta now, just release it... I'm not dying to release the next beta right now, but it'll be easier for other developers if there is a release with the ticket asap. -- You received this message because you are subscribed to the Google Groups sage-devel group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at http://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] Please review global docstring/test change #16746
I took a quick look and was daunted by the large number of changes, many in files I never normally look at. What's the best strategy -- apart from trusting you completely? Divide up the files and get a few people to look at them instead of just one? John On 23 September 2014 13:36, Volker Braun vbraun.n...@gmail.com wrote: bump On Thursday, September 18, 2014 9:27:18 PM UTC+1, Jeroen Demeyer wrote: On 2014-09-18 20:30, Volker Braun wrote: I would like to merge the ticket asap and then release the next beta. So please review ;-) I'm not convinced that you should postpone the next beta for this. If you think you have a candidate next beta now, just release it... I'm not dying to release the next beta right now, but it'll be easier for other developers if there is a release with the ticket asap. -- You received this message because you are subscribed to the Google Groups sage-devel group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at http://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups sage-devel group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at http://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] Please review global docstring/test change #16746
Trust me, I know what I'm doing ;-) The big piece is commit b579b92... which is mostly auto-generated changes to the existing doctests for the new output format. Its long but not really that exciting. You can see it with git show b579b92 The main work was done before that, to switch the doctesting framework around. To see the diff up to b579b92, you'd use git diff 6.4.beta3...b579b92~ (note: tilde = parent commit), which isn't all that much. After that there are a few fixups to issues raised on the ticket, which you can summarize with git diff 1a2b1cf...7de159a $ git log --oneline --graph trac/u/vbraun/sort_dicts_in_doctests * 7de159a Fix more random test failures * b362aa6 Display trailing newline in __repr__() output * 3922ed5 Reduce doctest precision * a9271f0 Fix doctests that have dictionary keys without stable order * fbf83eb Show output of the lonely generic_graph doctest * 48a2320 Also call graphics.show() in doctest mode * 1a2b1cf Add IPython formatting to the reference manual * b579b92 Update doctests that depended on the old displayhook * eb6a66b Refactor our displayhook IPython integration * a246e17 Fix printing of types * bdbdd5b Merge #16992 (Directly apply --fixdoctests) branch |\ | * 1b99c68 Also fix the cmdline doctest | * e397b3e Add newline at end of file and fix cmdline help | * da59f20 call git directly and from the right directory | * 8c5f2b0 Directly overwrite with fixed doctests | * 325da8b Updated Sage version to 6.4.beta3 On Tuesday, September 23, 2014 1:55:00 PM UTC+1, John Cremona wrote: I took a quick look and was daunted by the large number of changes, many in files I never normally look at. What's the best strategy -- apart from trusting you completely? Divide up the files and get a few people to look at them instead of just one? John On 23 September 2014 13:36, Volker Braun vbrau...@gmail.com javascript: wrote: bump On Thursday, September 18, 2014 9:27:18 PM UTC+1, Jeroen Demeyer wrote: On 2014-09-18 20:30, Volker Braun wrote: I would like to merge the ticket asap and then release the next beta. So please review ;-) I'm not convinced that you should postpone the next beta for this. If you think you have a candidate next beta now, just release it... I'm not dying to release the next beta right now, but it'll be easier for other developers if there is a release with the ticket asap. -- You received this message because you are subscribed to the Google Groups sage-devel group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com javascript:. To post to this group, send email to sage-...@googlegroups.com javascript:. Visit this group at http://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups sage-devel group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at http://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] Please review global docstring/test change #16746
On 2014-09-23 14:54, John Cremona wrote: I took a quick look and was daunted by the large number of changes, many in files I never normally look at. As I already said on the ticket, I checked those many changes and they are ok. What I have not checked is the actual code related to the displayhook. -- You received this message because you are subscribed to the Google Groups sage-devel group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at http://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] Please review global docstring/test change #16746
On 23 September 2014 17:23, Jeroen Demeyer jdeme...@cage.ugent.be wrote: On 2014-09-23 14:54, John Cremona wrote: I took a quick look and was daunted by the large number of changes, many in files I never normally look at. As I already said on the ticket, I checked those many changes and they are ok. What I have not checked is the actual code related to the displayhook. Well done.Surely if all those hunderds of lines now look good, that counts for a large number of tests that Volker did it right? If so, I think you can give the ticket a positive review... John -- You received this message because you are subscribed to the Google Groups sage-devel group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at http://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups sage-devel group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at http://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] Please review global docstring/test change #16746
On 2014-09-23 18:36, John Cremona wrote: Well done.Surely if all those hunderds of lines now look good, that counts for a large number of tests that Volker did it right? If so, I think you can give the ticket a positive review... I'm not so sure. For example, the last version of the patch accidentally disabled all doctests for plotting, a fact which could *easily* have been overlooked by a reviewer. I really would like somebody else (preferably somebody who knows about IPython and displayhooks) to review that part of the ticket. -- You received this message because you are subscribed to the Google Groups sage-devel group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at http://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
Re: [sage-devel] Please review global docstring/test change #16746
On 2014-09-18 20:30, Volker Braun wrote: I would like to merge the ticket asap and then release the next beta. So please review ;-) I'm not convinced that you should postpone the next beta for this. If you think you have a candidate next beta now, just release it... -- You received this message because you are subscribed to the Google Groups sage-devel group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at http://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
