-
-
Notifications
You must be signed in to change notification settings - Fork 481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
only use ASCII characters in patches #6674
Comments
comment:1
The patch
The attached patch Nathann: Can I ask you to review this? The ASCII character issue needs to be fixed before releasing Sage 4.1.1. |
Author: Minh Van Nguyen |
comment:3
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
ncohen: Do you have an account on sage.math? |
comment:6
I don't think so... I only have an account on TRAC, but I think that's all... What is sage.math ? ^^; |
comment:7
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 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
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:
I guess it also works with sphinx, but someone should try. Or is there a general policy i'm not aware of?! |
comment:9
This patch applies fine on my brand new install, but I have two remarks :
|
comment:10
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:
|
comment:11
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 ! |
comment:12
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
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. |
Attachment: trac_6674-use-ascii.patch.gz based on Sage 4.1.1.rc1 |
comment:14
Please refer to the new patch, which is based on Sage 4.1.1.rc1. |
comment:15
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 ! |
Merged: Sage 4.1.1.rc2 |
Reviewer: Nathann Cohen, Alex Ghitza |
This is a follow-up to #5793.
CC: @nathanncohen @rlmill
Component: documentation
Author: Minh Van Nguyen
Reviewer: Nathann Cohen, Alex Ghitza
Merged: Sage 4.1.1.rc2
Issue created by migration from https://trac.sagemath.org/ticket/6674
The text was updated successfully, but these errors were encountered: