Urlsplit does not accept unicode urls

[lopuhin]

Probably you are already aware, but anyway: urlparse4.urlsplit does not accept unicode:

In [4]: urlparse4.urlsplit(u'http://google.com')
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-7f5ea551e49e> in <module>()
----> 1 urlparse4.urlsplit(u'http://google.com')

urlparse4/cgurl.pyx in urlparse4.cgurl.urlsplit (urlparse4/cgurl.cpp:2537)()

TypeError: Argument 'url' has incorrect type (expected str, got unicode)

while urlparse.urlsplit from Python 2.7 does:

In [6]: urlparse.urlsplit(u'http://google.com')
Out[6]: SplitResult(scheme=u'http', netloc=u'google.com', path=u'', query='', fragment='')

[sylvinus]

Thanks! That should definitely be done if we want to pretend being a drop-in replacement :)

If we do a new API for #8 though, I think it should be utf8-only for performance, just like the underlying librairies.

[kmike]

@sylvinus do you mean bytes-only (aka utf8-only), or is it really ascii-only?

If it is the former, then for performance reasons it can be better to support unicode - otherwise users will have to encode/decode URLs themselves, and in Python url.encode('utf8') or url.decode('utf8') are much slower than in Cython. Cython compiles it directly to a C API calls (there is e.g. a special function to get utf-8 byte string from an unicode object), while in Python it must perform method dispatch, find a codec for utf8, etc.

[sylvinus]

Thanks @kmike for the clarification! I updated the comment, I did mean utf8-only.

My rationale for suggesting an utf8-only API is that in @commonsearch, the HTML data stays mostly in utf8 (from Common Crawl to Gumbo, language detection, ...) and we only convert to unicode for a fraction of the content (titles, ..).

That said, if users are aware of this and pass an unicode URL, we could convert it from the Cython side as you suggest, I don't think that would have a significant impact on the utf-8 only code path?

[kmike]

Yeah, the impact should be minimal. I haven't noticed any impact in project like marisa-trie - it has code like this, and as I recall the difference didn't show in benchmarks, unlike handling unicode-bytes-unicode conversion in Python:

def get(self, key, default=None):
    cdef bytes b_key
    if isinstance(key, unicode):
        # cython is (was?) not smart enough to infer types here properly,
        # without type conversions it generates(ed?) inefficient code
        b_key = <bytes>(<unicode>key).encode('utf8')
    else:
        b_key = key

Another option is to have two separate sets of methods, one which accepts bytes and another which accepts unicode.

It is true that most C libraries work in utf-8, but they can't work in unicode because there is no a common "unicode" object available in C/C++. In Python (at least in Python 3) most user-facing APIs return unicode. Returning unicode vs bytes is another question; maybe its effect worths benchmarking. If users want to work with unicode anyways then converting bytes to unicode in Cython can also speedup things quite a bit, and API becomes more convenient to use - no surprises like url.netloc == 'example.com' returning False in Python 3 and True in Python 2 because hostname is returned as bytes.

[kmike]

Tasks like detecting response encoding are non-trivial and easy to get wrong - even requests library doesn't mimic browser behavior properly (it doesn't take <meta> tags in account, sometimes retutning incorrect encoding for perfectly valid pages, so response.text is not always correct).

Many libraries expect utf8-encoded HTML regardless of original page encoding. Users must be aware of it: decode page content from its original encoding, then encode to utf8. With bytes API it is easy to get wrong and just pass the original page instead of utf8 version; it is much harder to make this mistake with unicode API.

Sorry, it seems I got side-tracked :)

[sylvinus]

Yep! I know this issue quite well. Common Search has code that mimics browser behaviour, including <meta> tags:
https://github.com/commonsearch/cosr-back/blob/master/cosrlib/document/html/htmlencoding.py

[kmike]

@sylvinus nice code! GUMBOCY_PARSER_HEAD is especially interesting.

In scrapy we're using w3lib utilities for that (https://github.com/scrapy/w3lib/blob/master/w3lib/encoding.py#L186). Maybe we should consider contributing/switching to webencodings - it seems that there is overlap, but these packages are not subsets of each other. There are encodings handled by webencodings, but not w3lib, and vice versa. w3lib also handles bom32.