Repository: pepkit/ubiquerg

Issue:
make is_url more strict?
Is the current implementation too lose?

```
In [1]: from ubiquerg import is_url

In [2]: is_url("description: 'this is a schema.'")
Out[2]: True
```

Here I also require no spaces:

```
def is_url(maybe_url):
    from urllib.parse import urlparse
    return ' ' not in maybe_url and urlparse(maybe_url).scheme != ""
```


```
In [4]:  is_url("description: 'this is a schema.'")
   ...: 
Out[4]: False
```


I"m passing yaml schemas and it's qualifying as `is_url`...


Discussion:

Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index 8fdc819..a0432ae 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -2,6 +2,11 @@
 
 This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html) and [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) format. 
 
+## [0.6.2] - 2021-01-28
+
+### Fixed 
+- `is_url` function; [Issue 32](https://github.com/pepkit/ubiquerg/issues/32)
+
 ## [0.6.1] - 2020-07-01
 
 ### Changed
diff --git a/setup.py b/setup.py
index 13d4aa6..93bd09d 100644
--- a/setup.py
+++ b/setup.py
@@ -8,29 +8,23 @@ REQDIR = "requirements"
 
 def read_reqs(reqs_name):
     depsfile = os.path.join(REQDIR, "requirements-{}.txt".format(reqs_name))
-    with open(depsfile, 'r') as f:
+    with open(depsfile, "r") as f:
         return [l.strip() for l in f if l.strip()]
 
 
-with open(os.path.join(PKG, "_version.py"), 'r') as versionfile:
+with open(os.path.join(PKG, "_version.py"), "r") as versionfile:
     version = versionfile.readline().split()[-1].strip("\"'\n")
 
-# Handle the pypi README (long description) formatting.
-try:
-    import pypandoc
-    long_description = pypandoc.convert_file("README.md", 'rst')
-    print("Pandoc conversion succeeded")
-except(IOError, ImportError, OSError):
-    print("Warning: pandoc conversion failed!")
-    long_description = open("README.md").read()
-
+with open("README.md") as f:
+    long_description = f.read()
+    
 setup(
     name=PKG,
     packages=[PKG],
     version=version,
     description="Various utility functions",
     long_description=long_description,
-    long_description_content_type='text/markdown', 
+    long_description_content_type="text/markdown",
     classifiers=[
         "Development Status :: 4 - Beta",
         "License :: OSI Approved :: BSD License",
@@ -46,5 +40,7 @@ setup(
     include_package_data=True,
     test_suite="tests",
     tests_require=read_reqs("dev"),
-    setup_requires=(["pytest-runner"] if {"test", "pytest", "ptr"} & set(sys.argv) else []),
+    setup_requires=(
+        ["pytest-runner"] if {"test", "pytest", "ptr"} & set(sys.argv) else []
+    ),
 )
diff --git a/ubiquerg/_version.py b/ubiquerg/_version.py
index 43c4ab0..22049ab 100644
--- a/ubiquerg/_version.py
+++ b/ubiquerg/_version.py
@@ -1,1 +1,1 @@
-__version__ = "0.6.1"
+__version__ = "0.6.2"
diff --git a/ubiquerg/cli_tools.py b/ubiquerg/cli_tools.py
index 48a5faf..8dcf156 100644
--- a/ubiquerg/cli_tools.py
+++ b/ubiquerg/cli_tools.py
@@ -1,8 +1,13 @@
 """ Functions for working with command-line interaction """
 
 from .collection import is_collection_like, merge_dicts
-from argparse import ArgumentParser, _SubParsersAction, _HelpAction, \
-    _VersionAction, SUPPRESS
+from argparse import (
+    ArgumentParser,
+    _SubParsersAction,
+    _HelpAction,
+    _VersionAction,
+    SUPPRESS,
+)
 import sys
 
 __classes__ = ["VersionInHelpParser"]
@@ -10,20 +15,25 @@ __all__ = __classes__ + ["build_cli_extra", "query_yes_no", "convert_value"]
 
 
 class VersionInHelpParser(ArgumentParser):
-
     def __init__(self, version=None, **kwargs):
-        """ Overwrites the inherited init. Saves the version as an object
-        attribute for further use. """
+        """Overwrites the inherited init. Saves the version as an object
+        attribute for further use."""
         super(VersionInHelpParser, self).__init__(**kwargs)
         self.version = version
         if self.version is not None:
-            self.add_argument('--version', action='version',
-                              version='%(prog)s {}'.format(self.version))
+            self.add_argument(
+                "--version",
+                action="version",
+                version="%(prog)s {}".format(self.version),
+            )
 
     def format_help(self):
         """ Add version information to help text. """
-        help_string = "version: {}\n".format(str(self.version)) \
-            if self.version is not None else ""
+        help_string = (
+            "version: {}\n".format(str(self.version))
+            if self.version is not None
+            else ""
+        )
         return help_string + super(VersionInHelpParser, self).format_help()
 
     def subparsers(self):
@@ -34,8 +44,7 @@ class VersionInHelpParser(ArgumentParser):
         """
         subs = [a for a in self._actions if isinstance(a, _SubParsersAction)]
         if len(subs) != 1:
-            raise ValueError(
-                "Expected exactly 1 subparser, got {}".format(len(subs)))
+            raise ValueError("Expected exactly 1 subparser, got {}".format(len(subs)))
         return subs[0]
 
     def top_level_args(self):
@@ -73,10 +82,16 @@ class VersionInHelpParser(ArgumentParser):
             return dest_list
 
         if subcommand is not None and subcommand not in self.subcommands():
-            raise ValueError("'{}' not found in this parser commands: {}".
-                             format(subcommand, str(self.subcommands())))
-        subs = self.subparsers().choices if subcommand is None \
+            raise ValueError(
+                "'{}' not found in this parser commands: {}".format(
+                    subcommand, str(self.subcommands())
+                )
+            )
+        subs = (
+            self.subparsers().choices
+            if subcommand is None
             else {subcommand: self.subparsers().choices[subcommand]}
+        )
         dests = {}
         for subcmd, sub in subs.items():
             dest_list = []
@@ -121,10 +136,16 @@ class VersionInHelpParser(ArgumentParser):
             return defaults_dict
 
         if subcommand is not None and subcommand not in self.subcommands():
-            raise ValueError("'{}' not found in this parser commands: {}".
-                             format(subcommand, str(self.subcommands())))
-        subs = self.subparsers().choices if subcommand is None \
+            raise ValueError(
+                "'{}' not found in this parser commands: {}".format(
+                    subcommand, str(self.subcommands())
+                )
+            )
+        subs = (
+            self.subparsers().choices
+            if subcommand is None
             else {subcommand: self.subparsers().choices[subcommand]}
+        )
         defaults = {}
         for subcmd, sub in subs.items():
             defaults_dict = {}
@@ -159,8 +180,7 @@ def build_cli_extra(optargs):
 
     def render(k, v):
         if not isinstance(k, str):
-            raise TypeError(
-                "Option name isn't a string: {} ({})".format(k, type(k)))
+            raise TypeError("Option name isn't a string: {} ({})".format(k, type(k)))
         if v is None:
             return k
         if is_collection_like(v):
@@ -183,12 +203,16 @@ def query_yes_no(question, default="no"):
     :param str default: the presumed answer if the user just hits <Enter>.
     :return bool: True for "yes" or False for "no"
     """
+
     def parse(ans):
-        return {"yes": True, "y": True, "ye": True,
-                "no": False, "n": False}[ans.lower()]
+        return {"yes": True, "y": True, "ye": True, "no": False, "n": False}[
+            ans.lower()
+        ]
+
     try:
-        prompt = {None: "[y/n]", "yes": "[Y/n]",
-                  "no": "[y/N]"}[None if default is None else default.lower()]
+        prompt = {None: "[y/n]", "yes": "[Y/n]", "no": "[y/N]"}[
+            None if default is None else default.lower()
+        ]
     except (AttributeError, KeyError):
         raise ValueError("invalid default answer: {}".format(default))
     msg = "{q} {p} ".format(q=question, p=prompt)
@@ -197,8 +221,7 @@ def query_yes_no(question, default="no"):
         try:
             return parse(_read_from_user() or default)
         except KeyError:
-            sys.stdout.write(
-                "Please respond with 'yes' or 'no' (or 'y' or 'n').\n")
+            sys.stdout.write("Please respond with 'yes' or 'no' (or 'y' or 'n').\n")
 
 
 def convert_value(val):
@@ -214,17 +237,19 @@ def convert_value(val):
         try:
             val = str(val)
         except:
-            raise ValueError("The input has to be of type convertible to 'str',"
-                             " got '{}'".format(type(val)))
+            raise ValueError(
+                "The input has to be of type convertible to 'str',"
+                " got '{}'".format(type(val))
+            )
 
     if isinstance(val, bool):
         return val
     if isinstance(val, str):
-        if val == 'None':
+        if val == "None":
             return None
-        if val.lower() == 'true':
+        if val.lower() == "true":
             return True
-        if val.lower() == 'false':
+        if val.lower() == "false":
             return False
         try:
             float(val)
@@ -241,7 +266,9 @@ def convert_value(val):
 
 def _read_from_user():
     import sys
+
     if sys.version_info.major < 3:
         from __builtin__ import raw_input
+
         return raw_input()
     return input()
diff --git a/ubiquerg/collection.py b/ubiquerg/collection.py
index 19d3663..2a442dc 100644
--- a/ubiquerg/collection.py
+++ b/ubiquerg/collection.py
@@ -3,6 +3,7 @@
 import itertools
 import sys
 from warnings import warn
+
 if sys.version_info < (3, 3):
     from collections import Iterable
 else:
@@ -72,11 +73,15 @@ def powerset(items, min_items=None, include_full_pop=True, nonempty=False):
         min_items = 1 if nonempty else 0
     else:
         if not isinstance(min_items, int):
-            raise TypeError("Min items count for each subset isn't an integer: "
-                            "{} ({})".format(min_items, type(min_items)))
+            raise TypeError(
+                "Min items count for each subset isn't an integer: "
+                "{} ({})".format(min_items, type(min_items))
+            )
         if nonempty and min_items < 1:
-            raise ValueError("When minimum item count is {}, nonempty subsets "
-                             "cannot be guaranteed.".format(min_items))
+            raise ValueError(
+                "When minimum item count is {}, nonempty subsets "
+                "cannot be guaranteed.".format(min_items)
+            )
     # Account for iterable burn possibility; besides, collection should be
     # relatively small if building the powerset.
     items = list(items)
@@ -84,9 +89,11 @@ def powerset(items, min_items=None, include_full_pop=True, nonempty=False):
     if n == 0 or n < min_items:
         return []
     max_items = len(items) + 1 if include_full_pop else len(items)
-    return list(itertools.chain.from_iterable(
-            itertools.combinations(items, k)
-            for k in range(min_items, max_items)))
+    return list(
+        itertools.chain.from_iterable(
+            itertools.combinations(items, k) for k in range(min_items, max_items)
+        )
+    )
 
 
 def asciify_dict(data):
@@ -96,7 +103,7 @@ def asciify_dict(data):
         ret = []
         for item in lst:
             if isinstance(item, unicode):
-                item = item.encode('utf-8')
+                item = item.encode("utf-8")
             elif isinstance(item, list):
                 item = _asciify_list(item)
             elif isinstance(item, dict):
@@ -111,12 +118,12 @@ def asciify_dict(data):
     ret = {}
     for key, value in data.iteritems():
         if isinstance(key, unicode):
-            key = key.encode('utf-8')
+            key = key.encode("utf-8")
         if isinstance(value, unicode):
-            value = value.encode('utf-8')
+            value = value.encode("utf-8")
         elif isinstance(value, list):
             value = _asciify_list(value)
         elif isinstance(value, dict):
             value = asciify_dict(value)
         ret[key] = value
-    return ret
\ No newline at end of file
+    return ret
diff --git a/ubiquerg/environment.py b/ubiquerg/environment.py
index ec41554..ed27a18 100644
--- a/ubiquerg/environment.py
+++ b/ubiquerg/environment.py
@@ -10,12 +10,14 @@ __all__ = ["TmpEnv"]
 
 class TmpEnv(object):
     """ Temporary environment variable setting. """
+
     def __init__(self, overwrite=False, **kwargs):
         if not overwrite:
             already_set = [k for k, v in kwargs.items() if os.getenv(k, v) != v]
             if already_set:
-                msg = "{} variable(s) already set: {}".\
-                    format(len(already_set), ", ".join(already_set))
+                msg = "{} variable(s) already set: {}".format(
+                    len(already_set), ", ".join(already_set)
+                )
                 raise ValueError(msg)
         self._kvs = kwargs
 
diff --git a/ubiquerg/files.py b/ubiquerg/files.py
index ed26b2c..fc8e53a 100644
--- a/ubiquerg/files.py
+++ b/ubiquerg/files.py
@@ -9,14 +9,22 @@ from warnings import warn
 from tarfile import open as topen
 from hashlib import md5
 
-__all__ = ["checksum", "size", "filesize_to_str", "untar", "create_lock",
-           "remove_lock", "wait_for_lock", "create_file_racefree",
-           "make_lock_path"]
-FILE_SIZE_UNITS = ['B', 'KB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB']
+__all__ = [
+    "checksum",
+    "size",
+    "filesize_to_str",
+    "untar",
+    "create_lock",
+    "remove_lock",
+    "wait_for_lock",
+    "create_file_racefree",
+    "make_lock_path",
+]
+FILE_SIZE_UNITS = ["B", "KB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"]
 LOCK_PREFIX = "lock."
 
 
-def checksum(path, blocksize=int(2e+9)):
+def checksum(path, blocksize=int(2e9)):
     """
     Generate a md5 checksum for the file contents in the provided path.
 
@@ -25,7 +33,7 @@ def checksum(path, blocksize=int(2e+9)):
     :return str: checksum hash
     """
     m = md5()
-    with open(path, 'rb') as f:
+    with open(path, "rb") as f:
         while True:
             buf = f.read(blocksize)
             if not buf:
@@ -63,8 +71,9 @@ def size(path, size_str=True):
                     s += os.lstat(fp).st_size
                     symlinks.append(fp)
         if len(symlinks) > 0:
-            print("{} symlinks were found: {}".format(len(symlinks),
-                                                      "\n".join(symlinks)))
+            print(
+                "{} symlinks were found: {}".format(len(symlinks), "\n".join(symlinks))
+            )
     else:
         warn("size could not be determined for: {}".format(path))
         s = None
@@ -83,8 +92,9 @@ def filesize_to_str(size):
             if size < 1024:
                 return "{}{}".format(round(size, 1), unit)
             size /= 1024
-    warn("size argument was neither an int nor a float, "
-         "returning the original object")
+    warn(
+        "size argument was neither an int nor a float, " "returning the original object"
+    )
     return size
 
 
@@ -111,8 +121,10 @@ def get_file_mod_time(pth):
     try:
         return os.path.getmtime(pth)
     except Exception as e:
-        print("Could not determine timestamp for '{}'. Returning current time. "
-              "Caught exception: {}".format(pth, getattr(e, 'message', repr(e))))
+        print(
+            "Could not determine timestamp for '{}'. Returning current time. "
+            "Caught exception: {}".format(pth, getattr(e, "message", repr(e)))
+        )
         return time.time()
 
 
@@ -123,7 +135,7 @@ def wait_for_lock(lock_file, wait_max=30):
     :param str lock_file: Lock file to wait upon.
     :param int wait_max: max wait time if the file in question is already locked
     """
-    sleeptime = .001
+    sleeptime = 0.001
     first_message_flag = False
     dot_count = 0
     totaltime = 0
@@ -142,7 +154,7 @@ def wait_for_lock(lock_file, wait_max=30):
         sys.stdout.flush()
         time.sleep(sleeptime)
         totaltime += sleeptime
-        sleeptime = min((sleeptime + .1) * 1.25, 10)
+        sleeptime = min((sleeptime + 0.1) * 1.25, 10)
         if totaltime >= wait_max:
             if os.path.isfile(lock_file):
                 timestamp = get_file_mod_time(lock_file)
@@ -153,7 +165,8 @@ def wait_for_lock(lock_file, wait_max=30):
                     continue
                 raise RuntimeError(
                     "The maximum wait time ({}) has been reached and the lock "
-                    "file still exists.".format(wait_max))
+                    "file still exists.".format(wait_max)
+                )
     if first_message_flag:
         print(" File unlocked")
 
@@ -182,12 +195,17 @@ def make_lock_path(lock_name_base):
     :param str | list[str] lock_name_base: Lock file names
     :return str | list[str]: Path to the lock files.
     """
+
     def _mk_lock(lnb):
         base, name = os.path.split(lnb)
         lock_name = name if name.startswith(LOCK_PREFIX) else LOCK_PREFIX + name
         return lock_name if not base else os.path.join(base, lock_name)
-    return [_mk_lock(x) for x in lock_name_base] \
-        if isinstance(lock_name_base, list) else _mk_lock(lock_name_base)
+
+    return (
+        [_mk_lock(x) for x in lock_name_base]
+        if isinstance(lock_name_base, list)
+        else _mk_lock(lock_name_base)
+    )
 
 
 def remove_lock(filepath):
@@ -221,7 +239,8 @@ def _create_lock(lock_path, filepath, wait_max):
             # `wait_max`.
             print(
                 "The lock has been created in the split second since the "
-                "last lock existence check. Waiting")
+                "last lock existence check. Waiting"
+            )
             wait_for_lock(lock_path, wait_max)
             _create_lock(lock_path, filepath, wait_max)
         else:
diff --git a/ubiquerg/paths.py b/ubiquerg/paths.py
index 5d941d3..0db9e3a 100644
--- a/ubiquerg/paths.py
+++ b/ubiquerg/paths.py
@@ -2,7 +2,6 @@
 
 import os
 import re
-from .web import is_url
 
 __author__ = "Vince Reuter"
 __email__ = "vreuter@virginia.edu"
@@ -20,14 +19,16 @@ def expandpath(path):
     return os.path.expandvars(os.path.expanduser(path))
 
 
-def parse_registry_path(rpstring,
-                        defaults=[
-                            ("protocol", None),
-                            ("namespace", None),
-                            ("item", None),
-                            ("subitem", None),
-                            ("tag", None)]
-                        ):
+def parse_registry_path(
+    rpstring,
+    defaults=[
+        ("protocol", None),
+        ("namespace", None),
+        ("item", None),
+        ("subitem", None),
+        ("tag", None),
+    ],
+):
     """
     Parse a 'registry path' string into components.
 
@@ -46,7 +47,7 @@ def parse_registry_path(rpstring,
     # This commented regex is the same without protocol
     # ^(?:([0-9a-zA-Z_-]+)\/)?([0-9a-zA-Z_-]+)(?::([0-9a-zA-Z_.-]+))?$
     # regex = "^(?:([0-9a-zA-Z_-]+)(?:::|:\/\/))?(?:([0-9a-zA-Z_-]+)\/)?([0-9a-zA-Z_-]+)(?::([0-9a-zA-Z_.-]+))?$"
-    regex = "^(?:([0-9a-zA-Z_-]+)(?:::|:\/\/))?(?:([0-9a-zA-Z_-]+)\/)?([0-9a-zA-Z_-]+)(?:\.([0-9a-zA-Z_-]+))?(?::([0-9a-zA-Z_.-]+))?$"
+    regex = "^(?:([0-9a-zA-Z_-]+)(?:::|:\/\/))?(?:([0-9a-zA-Z_-]+)\/)?([0-9a-zA-Z_-]+)(?:\.([0-9a-zA-Z_-]+))?(?::([0-9a-zA-Z_.,|+()-]+))?$"
     # This regex matches strings like:
     # protocol://namespace/item:tag
     # or: protocol::namespace/item:tag
@@ -73,7 +74,7 @@ def parse_registry_path(rpstring,
         defaults[1][0]: captures[1] or defaults[1][1],
         defaults[2][0]: captures[2] or defaults[2][1],
         defaults[3][0]: captures[3] or defaults[3][1],
-        defaults[4][0]: captures[4] or defaults[4][1]
+        defaults[4][0]: captures[4] or defaults[4][1],
     }
     return parsed_identifier
 
@@ -90,6 +91,7 @@ def mkabs(path, reldir=None):
 
     :return str: Absolute path
     """
+
     def xpand(path):
         return os.path.expandvars(os.path.expanduser(path))
 
diff --git a/ubiquerg/web.py b/ubiquerg/web.py
index 8ea7d10..e16037d 100644
--- a/ubiquerg/web.py
+++ b/ubiquerg/web.py
@@ -1,14 +1,6 @@
 """ Web-related utilities """
 
-import sys
-if sys.version_info < (3, 0):
-    from urlparse import urlparse
-else:
-    from urllib.parse import urlparse
-
-
-__author__ = "Vince Reuter"
-__email__ = "vreuter@virginia.edu"
+import re
 
 __all__ = ["is_url"]
 
@@ -20,4 +12,15 @@ def is_url(maybe_url):
     :param str maybe_url: path to investigate as URL
     :return bool: whether path appears to be a URL
     """
-    return urlparse(maybe_url).scheme != ""
+    # from Django 1.3.x
+    # https://github.com/django/django/blob/6726d750979a7c29e0dd866b4ea367eef7c8a420/django/core/validators.py#L45-L51
+    regex = re.compile(
+        r"^(?:http|ftp)s?://"
+        r"(?:(?:[A-Z0-9](?:[A-Z0-9-]{0,61}[A-Z0-9])?\.)+(?:[A-Z]{2,6}\.?|[A-Z0-9-]{2,}\.?)|"
+        r"localhost|"
+        r"\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})"
+        r"(?::\d+)?"
+        r"(?:/?|[/?]\S+)$",
+        re.IGNORECASE,
+    )
+    return re.match(regex, maybe_url) is not None


Test patch:
diff --git a/tests/test_web.py b/tests/test_web.py
new file mode 100644
index 0000000..0d0664f
--- /dev/null
+++ b/tests/test_web.py
@@ -0,0 +1,19 @@
+""" Tests for web """
+
+import pytest
+from ubiquerg.web import is_url
+
+
+@pytest.mark.parametrize("s", ["https://www.github.com",
+                                    "https://www.youtube.com"]
+                         )
+def test_is_url_tests_positive(s):
+    assert is_url(s)
+
+
+@pytest.mark.parametrize("s", ["www.github.com",
+                                    "test: string spaces",
+                                    "j%2vv@::https://test.com"]
+                         )
+def test_is_url_tests_negative(s):
+    assert not is_url(s)