Opened 12 years ago

Closed 12 years ago

#6674 closed defect (fixed)

[with patch, positive review] only use ASCII characters in patches

Reported by: mvngu Owned by: tba
Priority: blocker Milestone: sage-4.1.1
Component: documentation Keywords:
Cc: ncohen, rlm Merged in: Sage 4.1.1.rc2
Authors: Minh Van Nguyen Reviewers: Nathann Cohen, Alex Ghitza
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

This is a follow-up to #5793.

Attachments (1)

trac_6674-use-ascii.patch (4.7 KB) - added by mvngu 12 years ago.
based on Sage 4.1.1.rc1

Download all attachments as: .zip

Change History (17)

comment:1 Changed 12 years ago by mvngu

  • Authors set to Minh Van Nguyen
  • Priority changed from major to blocker
  • Summary changed from only use ASCII characters in patches to [with patch, needs review] only use ASCII characters in patches

The patch trac_5793-cliquer-flattened.patch at #5793 contains non-ASCII characters. This can result in errors and warning messages when loading Sage. Patches should only use ASCII characters. The offending characters from that patch are

Östergård

The attached patch trac_6674-use-ascii.patch changes that to "Ostergard". It also fixes referencing so that [NisOst2003] appears as a hyperlink in the HTML version of the reference manual, as it should be. This patch is based on Sage 4.1.1.rc0 and depends on #5793.

Nathann: Can I ask you to review this? The ASCII character issue needs to be fixed before releasing Sage 4.1.1.

comment:2 Changed 12 years ago by mvngu

  • Component changed from algebra to documentation
  • Owner changed from tbd to tba

comment:3 Changed 12 years ago by ncohen

Hello !!

Actually, I can not apply this patch as I have nothing in my graph.py similar to what your patch tries to replace. I may be using some different version of cliquer. And I look through the patches send by rlm without finding where I stopped

I am currently downloading http://ftp.sh.cvut.cz/MIRRORS/sagemath/src/sage-4.1.tar which I will then compile and upgrade to test this patch. However, it is likely that it will take something like 6~7 hours and I will not be able to give you an answer until then.... Sorry !!! ;-)

comment:4 Changed 12 years ago by mvngu

ncohen: Do you have an account on sage.math?

comment:5 Changed 12 years ago by mvngu

  • Cc rlm added

comment:6 Changed 12 years ago by ncohen

I don't think so... I only have an account on TRAC, but I think that's all... What is sage.math ? ;

comment:7 Changed 12 years ago by mvngu

The machine sage.math is mostly used for Sage development. You can browse the account holders' home directories at

http://sage.math.washington.edu/home/

In particular, my development directory is

http://sage.math.washington.edu/home/mvngu/

If you have an account on sage.math, you can get all source and binary releases of the 4.1.1 release cycle at

http://sage.math.washington.edu/home/mvngu/release/

in particular the binary

http://sage.math.washington.edu/home/mvngu/release/sage-4.1.1.rc0-sage.math.washington.edu-x86_64-Linux.tar.gz

which has been specifically compiled for sage.math. If you don't have an account on sage.math, you can email William Stein about getting an account on that machine.

comment:8 Changed 12 years ago by schilly

Uhm, I question this a litte bit. It's true that you have to use ASCII only for variable names, but strings and comments can be UTF-8. At least it is technically feasible. You just have to add this line in the first or second one of the .py file:

# -*- coding: utf-8 -*-

I guess it also works with sphinx, but someone should try.

Or is there a general policy i'm not aware of?!

comment:9 Changed 12 years ago by ncohen

This patch applies fine on my brand new install, but I have two remarks :

  • I received no error message nor any warning when starting Sage with the Non-ascii characters and using the corresponding functions, so my report may not be relevant
  • There are more occurencies of Ostergard in the following functions : cliques_maximum and cliques_maximal

comment:10 Changed 12 years ago by mvngu

The new patch should have removed all non-ASCII characters. It also removes duplicate citations. That is, don't redefine a reference for each function. Only define a reference once. If you then need to refer to that reference, then use a shorthand notation to refer to that reference. That way, it wouldn't result in warnings when building the reference manual.

Here are the steps to see the warnings caused by patches or library files with non-ASCII characters:

  1. Take a binary or source version of Sage 4.1.1.rc0.
  2. Apply the patches trac_5793-cliquer-flattened.patch and trac_5793-part-6.patch at ticket #5793. Install the package http://sage.math.washington.edu/home/mvngu/patch/cliquer-1.2.spkg .
  3. Run Sage with ./sage -br main and exit Sage.
  4. Make an experimental binary version of the patched repository with ./sage -bdist 4.1.1.rc0-exp. The resulting binary can be found in SAGE_ROOT/dist.
  5. Extract the resulting binary and run it with ./sage. You would see something similar to:
    [mvngu@sage sage-4.1.1.rc0-sage.math-x86_64-Linux]$ ./sage
    ----------------------------------------------------------------------
    | Sage Version 4.1.1.rc0, Release Date: 2009-07-29                   |
    | Type notebook() for the GUI, and license() for information.        |
    ----------------------------------------------------------------------
    **********************************************************************
    *                                                                    *
    * Warning: this is a prerelease version, and it may be unstable.     *
    *                                                                    *
    **********************************************************************
    The Sage install tree may have moved.
    Regenerating Python.pyo and .pyc files that hardcode the install PATH
    (please wait at most a few minutes)...
    Do not interrupt this.
    ---------------------------------------------------------------------------
    SyntaxError                               Traceback (most recent call last)
    
    /scratch/mvngu/sage-4.1.1.rc0-sage.math-x86_64-Linux/local/lib/python2.6/site-packages/IPython/ipmaker.py in force_import(modname)
         64         reload(sys.modules[modname])
         65     else:
    ---> 66         __import__(modname)
         67 
         68 
    
    /scratch/mvngu/sage-4.1.1.rc0-sage.math-x86_64-Linux/local/bin/ipy_profile_sage.py in <module>()
          5     preparser(True)
          6 
    ----> 7     import sage.all_cmdline
          8     sage.all_cmdline._init_cmdline(globals())
          9 
    
    /scratch/mvngu/sage-4.1.1.rc0-sage.math-x86_64-Linux/local/lib/python2.6/site-packages/sage/all_cmdline.py in <module>()
         12 try:
         13 
    ---> 14     from sage.all import *
         15     from sage.calculus.predefined import x
         16     preparser(on=True)
    
    /scratch/mvngu/sage-4.1.1.rc0-sage.math-x86_64-Linux/local/lib/python2.6/site-packages/sage/all.py in <module>()
         83 from sage.modular.all    import *
         84 from sage.schemes.all    import *
    ---> 85 from sage.graphs.all     import *
         86 from sage.groups.all     import *
         87 from sage.databases.all  import *
    
    /scratch/mvngu/sage-4.1.1.rc0-sage.math-x86_64-Linux/local/lib/python2.6/site-packages/sage/graphs/all.py in <module>()
    ----> 1 
          2 
          3 from graph_generators import graphs, digraphs
          4 from graph_database import GraphDatabase, GenericGraphQuery, GraphQuery
          5 from graph import Graph, DiGraph
          6 from bipartite_graph import BipartiteGraph
          7 from graph_bundle import GraphBundle
    
    /scratch/mvngu/sage-4.1.1.rc0-sage.math-x86_64-Linux/local/lib/python2.6/site-packages/sage/graphs/graph_generators.py in <module>()
        139 ################################################################################
    
        140 
    --> 141 import graph
        142 from   math import sin, cos, pi
        143 from sage.misc.randstate import current_randstate
    
    SyntaxError: Non-ASCII character '\xc3' in file /scratch/mvngu/sage-4.1.1.rc0-sage.math-x86_64-Linux/local/lib/python2.6/site-packages/sage/graphs/graph.py on line 10146, but no encoding declared; see http://www.python.org/peps/pep-0263.html for details (graph.py, line 10145)
    Error importing ipy_profile_sage - perhaps you should run %upgrade?
    WARNING: Loading of ipy_profile_sage failed.
    

comment:11 Changed 12 years ago by ncohen

This patch fails to apply on my version, and I am afraid I do not know where it comes from. As I still have no answer from William Stein for an account on sage.math, I hope somebody else will be able to try it ! The odd thing is that I am asked to enter the "list of the changes" when I apply the patch, as if I were committing it !

comment:12 Changed 12 years ago by ncohen

  • Summary changed from [with patch, needs review] only use ASCII characters in patches to [with patch, positive review] only use ASCII characters in patches

Perfect ! I was finally able to retry this patch on sage 4.1.1.rc0 on which it applied without any problem. I then built and started a binary version as described and no errors where reported.

Positive review !

comment:13 Changed 12 years ago by AlexGhitza

I don't have a strong opinion about this, but given the discussion about how to ASCII-cise that name, I looked at the README file of the cliquer package, and the copyright notice has "Patric Ostergard". So it seems that the author himself has chosen this as the ASCII version of his name. It might be good to go with that.

Changed 12 years ago by mvngu

based on Sage 4.1.1.rc1

comment:14 Changed 12 years ago by mvngu

  • Summary changed from [with patch, positive review] only use ASCII characters in patches to [with patch, needs review] only use ASCII characters in patches

Please refer to the new patch, which is based on Sage 4.1.1.rc1.

comment:15 Changed 12 years ago by ncohen

  • Summary changed from [with patch, needs review] only use ASCII characters in patches to [with patch, positive review] only use ASCII characters in patches

I applied this on a 4.1.1.rc1, which refused to start before this patch was applied on it. I applied it from the command line using hg import, then re-built Sage.

Nothing wrong to report. Positive review !

comment:16 Changed 12 years ago by mvngu

  • Merged in set to Sage 4.1.1.rc2
  • Resolution set to fixed
  • Reviewers set to Nathann Cohen, Alex Ghitza
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.