Changes between Version 114 and Version 116 of Ticket #7377

03/14/11 17:00:08 (8 years ago)

Sorry I do not have time to check this properly today. Do you get the

ERROR: An unexpected error occurred while tokenizing input
The following traceback may be corrupted or invalid

problem with these patches? I hope not :)

A couple very small points about the doctests patch, which by the way is nontrivial and nice all by itself.

  • You should have a loads(dumps(foo))==foo test if possible for class definitions. I think there are eight of these?
  • You probably don't need to put INPUT and/or OUTPUT blocks if there isn't one or it's essentially stated in the first line of the documentation - I'm thinking of _false_symbol or version. Up to you, of course, since you already did it.
  • In a few places where you have more than 'really basic and stupid' documentation (you sell yourself short! it's not bad at all) you have some lines that aren't going to work. Ones like
    - ``use_disk_cache`` - boolean (default: True); if set to True, try to read cached result from disk 

are too long, and should be made into two lines like you did for

- ``eqns`` - a list of m strings; each representing a linear 
question in m = n variables 

However, these will cause a problem in Sphinx, I think, unless you reformat them like so

- ``eqns`` - a list of m strings; each representing a linear 
  question in m = n variables 

This won't show up well on Trac, but notice that the 'q' in 'question' is lined up with the first '' in 'eqns`'. This was correctly done for 'pts_list', for example.

It looks like the zunderflow zz was fixed as well, and assuming that fix passes tests as well, I think it can be 'needs review'.

In fact, as long as everyone involved is not reviewing their own patch, this can even be 'positive review'. For instance, I will assume that JP slightly changing my reviewer patch indicates no problems with it.

Can anyone say what would still need to be done for positive review, other than passing all long doctests, which I can't check right now, and taking care of the things above which will lead to Sphinx errors?


  • Ticket #7377

    • Property Status changed from needs_work to needs_review
  • Ticket #7377 – Description

    v114 v116  
    16 trac_7377-assumptions-p1.patch
    2525 * attachment:trac_7377-floatcast.patch
    2626 * attachment:trac_7377-unicode_to_ecl-p1.patch
    27  * attachment:trac_7377-assumptions-p1.patch
     27 * attachment:trac_7377-assumptions-p2.patch
    2828 * attachment:trac_7377-doctests.patch