"Martin v. Loewis" wrote:

...

You should make this: 'ascii' -- encoding names are lower case per convention (and the implementation has a short-cut to speed up conversion to 'ascii' -- not for 'ASCII').

With conventions, it is a difficult story. I'm pretty certain that users typically see that particular american standard as ASCII (to the extend of calling it "a s c two"), not ascii.

It's a convention in the codec registry design and used as such in the Unicode implementation.

As for speed - feel free to change the code if you think it matters.

Hey... this was just a suggestion. I thought that you didn't know of the internal short-cut and wanted to hint at it.

...

+ raise UnicodeError("URL "+repr(url)+" contains non-ASCII characters")

...

Would it be better to use a simple ValueError here ? (UnicodeError is a subclass of ValueError, but the error doesn't really have something to do with Unicode conversions...)

Why does it not have to do with Unicode conversion? A conversion from Unicode to ASCII was attempted, and failed.

Sure, but the fact that URLs have to be ASCII is not something that is enforced by the Unicode implementation.

I guess I would be more open to suggested changes if you had put them into the patch manager at the time you've reviewed the patch...

I didn't review the patch, only the summary... Don't have much time to look into these things closely right now, so all I can do is comment. -- Marc-Andre Lemburg ______________________________________________________________________ Company: http://www.egenix.com/ Consulting: http://www.lemburg.com/ Python Pages: http://www.lemburg.com/python/

"Martin v. Loewis" wrote:

...

You should make this: 'ascii' -- encoding names are lower case per convention (and the implementation has a short-cut to speed up conversion to 'ascii' -- not for 'ASCII').

With conventions, it is a difficult story. I'm pretty certain that users typically see that particular american standard as ASCII (to the extend of calling it "a s c two"), not ascii.

It's a convention in the codec registry design and used as such in the Unicode implementation.

As for speed - feel free to change the code if you think it matters.

Hey... this was just a suggestion. I thought that you didn't know of the internal short-cut and wanted to hint at it.

...

+ raise UnicodeError("URL "+repr(url)+" contains non-ASCII characters")

...

Would it be better to use a simple ValueError here ? (UnicodeError is a subclass of ValueError, but the error doesn't really have something to do with Unicode conversions...)

Why does it not have to do with Unicode conversion? A conversion from Unicode to ASCII was attempted, and failed.

Sure, but the fact that URLs have to be ASCII is not something that is enforced by the Unicode implementation.

I guess I would be more open to suggested changes if you had put them into the patch manager at the time you've reviewed the patch...

I didn't review the patch, only the summary... Don't have much time to look into these things closely right now, so all I can do is comment. -- Marc-Andre Lemburg ______________________________________________________________________ Company: http://www.egenix.com/ Consulting: http://www.lemburg.com/ Python Pages: http://www.lemburg.com/python/