Add __all__ to w3lib.url package #55
Conversation
Current coverage is
|
| @@ -264,3 +264,8 @@ def any_to_uri(uri_or_path): | |||
| return path_to_file_uri(uri_or_path) | |||
| u = urlparse(uri_or_path) | |||
| return uri_or_path if u.scheme else path_to_file_uri(uri_or_path) | |||
|
|
|||
|
|
|||
| __all__ = ["add_or_replace_parameter", "any_to_uri", "file_uri_to_path", | |||
There was a problem hiding this comment.
what do you think about adding deprecated and semi-private function here as well?
urljoin_rfc, is_url, url_query_parameter are missing.
There was a problem hiding this comment.
url_query_parameter: my bad, I missed this one when looking at https://w3lib.readthedocs.org/en/latest/w3lib.html#module-w3lib.urlis_urlis used in scrapy even, so makes sense to add, although it's not documented.- I'm not sure about
urljoin_rfc, I'd prefer not, even if it's documented.
any other you had in mind?
There was a problem hiding this comment.
(ok, seems I did not push latest local changes)
There was a problem hiding this comment.
Exposing variables like _safe_chars could have made sense, but import * doesn't import them anyways, so no need to add them to __all__.
What's wrong with exposing urljoin_rfc? Not exposing it may break some code. The whole __all__ fix is needed for backwards compatibility, so why fix it for some functions but not with others?
There was a problem hiding this comment.
Nothing wrong, just that "exposing" is explicitly may keep people using it while we don't recommend it.
I added it anyway
Fixes #54