Repository: spacetx/slicedimage

Issue:
slicedimage relative paths in hybridization.json are relative to experiment.json
For example, in the following file structure: 
```
experiment.json
fov_001/
  hybridization.json/
  file_0.tiff
```
The file path to `file_0.tiff` in `hybridization.json` needs to be `fov_001/file_0.tiff` _not_ `file_0.tiff`, which is counter-intuitive. 

The below code reproduces this issue -- it loads, but contains the odd file paths suggested above. 

```
experiment = Experiment()
experiment.read('http://czi.starfish.data.public.s3.amazonaws.com/20180813/MERFISH/experiment.json')
```

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/spacetx/slicedimage/pull/26?src=pr&el=h1) Report
> Merging [#26](https://codecov.io/gh/spacetx/slicedimage/pull/26?src=pr&el=desc) into [master](https://codecov.io/gh/spacetx/slicedimage/commit/a4bcd1715016ca1411d70cdd728ed53be411a341?src=pr&el=desc) will **increase** coverage by `1.8%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/spacetx/slicedimage/pull/26/graphs/tree.svg?token=h4lI8HE0K6&width=650&height=150&src=pr)](https://codecov.io/gh/spacetx/slicedimage/pull/26?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##           master      #26     +/-   ##
=========================================
+ Coverage   74.84%   76.64%   +1.8%     
=========================================
  Files          18       18             
  Lines         469      471      +2     
=========================================
+ Hits          351      361     +10     
+ Misses        118      110      -8
```


| [Impacted Files](https://codecov.io/gh/spacetx/slicedimage/pull/26?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [slicedimage/io.py](https://codecov.io/gh/spacetx/slicedimage/pull/26/diff?src=pr&el=tree#diff-c2xpY2VkaW1hZ2UvaW8ucHk=) | `92.63% <100%> (+4.33%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/spacetx/slicedimage/pull/26?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/spacetx/slicedimage/pull/26?src=pr&el=footer). Last update [a4bcd17...6628851](https://codecov.io/gh/spacetx/slicedimage/pull/26?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/slicedimage/io.py b/slicedimage/io.py
index f930798..afe39c6 100644
--- a/slicedimage/io.py
+++ b/slicedimage/io.py
@@ -9,7 +9,7 @@ import tempfile
 from packaging import version
 from six.moves import urllib
 
-from slicedimage.urlpath import pathsplit
+from slicedimage.urlpath import pathjoin, pathsplit
 from .backends import DiskBackend, HttpBackend
 from ._collection import Collection
 from ._formats import ImageFormat
@@ -58,26 +58,35 @@ def resolve_path_or_url(path_or_url, allow_caching=True):
         raise
 
 
+def _resolve_absolute_url(absolute_url, allow_caching):
+    """
+    Given a string that is an absolute URL, return a tuple consisting of: a
+    :py:class:`slicedimage.backends._base.Backend`, the basename of the object, and the baseurl of
+    the object.
+    """
+    splitted = pathsplit(absolute_url)
+    backend = infer_backend(splitted[0], allow_caching)
+    return backend, splitted[1], splitted[0]
+
+
 def resolve_url(name_or_url, baseurl=None, allow_caching=True):
     """
     Given a string that can either be a name or a fully qualified url, return a tuple consisting of:
     a :py:class:`slicedimage.backends._base.Backend`, the basename of the object, and the baseurl of
     the object.
 
-    If the string is a name and not a fully qualified url, then baseurl must be set.
+    If the string is a name and not a fully qualified url, then baseurl must be set.  If the string
+    is a fully qualified url, then baseurl is ignored.
     """
     try:
         # assume it's a fully qualified url.
-        splitted = pathsplit(name_or_url)
-        backend = infer_backend(splitted[0], allow_caching)
-        return backend, splitted[1], splitted[0]
+        return _resolve_absolute_url(name_or_url, allow_caching)
     except ValueError:
         if baseurl is None:
             # oh, we have no baseurl.  punt.
             raise
-        # it's not a fully qualified url.
-        backend = infer_backend(baseurl, allow_caching)
-        return backend, name_or_url, baseurl
+        absolute_url = pathjoin(baseurl, name_or_url)
+        return _resolve_absolute_url(absolute_url, allow_caching)
 
 
 class Reader(object):


Test patch:
diff --git a/tests/io/test_resolve_url.py b/tests/io/test_resolve_url.py
new file mode 100644
index 0000000..c39152f
--- /dev/null
+++ b/tests/io/test_resolve_url.py
@@ -0,0 +1,65 @@
+import os
+import sys
+import tempfile
+import unittest
+import uuid
+
+
+pkg_root = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))  # noqa
+sys.path.insert(0, pkg_root)  # noqa
+
+
+from slicedimage.io import resolve_path_or_url, resolve_url
+
+
+class TestResolvePathOrUrl(unittest.TestCase):
+    def test_valid_local_path(self):
+        with tempfile.NamedTemporaryFile() as tfn:
+            abspath = os.path.realpath(tfn.name)
+            _, name, baseurl = resolve_path_or_url(abspath)
+            self.assertEqual(name, os.path.basename(abspath))
+            self.assertEqual("file://{}".format(os.path.dirname(abspath)), baseurl)
+
+            cwd = os.getcwd()
+            try:
+                os.chdir(os.path.dirname(abspath))
+                _, name, baseurl = resolve_path_or_url(os.path.basename(abspath))
+                self.assertEqual(name, os.path.basename(abspath))
+                self.assertEqual("file://{}".format(os.path.dirname(abspath)), baseurl)
+            finally:
+                os.chdir(cwd)
+
+    def test_invalid_local_path(self):
+        with self.assertRaises(ValueError):
+            resolve_path_or_url(str(uuid.uuid4()))
+
+    def test_url(self):
+        _, name, baseurl = resolve_path_or_url("https://github.com/abc/def")
+        self.assertEqual(name, "def")
+        self.assertEqual(baseurl, "https://github.com/abc")
+
+
+class TestResolveUrl(unittest.TestCase):
+    def test_fully_qualified_url(self):
+        _, name, baseurl = resolve_url("https://github.com/abc/def")
+        self.assertEqual(name, "def")
+        self.assertEqual(baseurl, "https://github.com/abc")
+
+        # even with a baseurl, this should work.
+        _, name, baseurl = resolve_url("https://github.com/abc/def", "https://github.io")
+        self.assertEqual(name, "def")
+        self.assertEqual(baseurl, "https://github.com/abc")
+
+    def test_relative_url(self):
+        _, name, baseurl = resolve_url("def", "https://github.com/abc")
+        self.assertEqual(name, "def")
+        self.assertEqual(baseurl, "https://github.com/abc")
+
+        # even with a path separator in the relative path, it should work.
+        _, name, baseurl = resolve_url("abc/def", "https://github.com/")
+        self.assertEqual(name, "def")
+        self.assertEqual(baseurl, "https://github.com/abc")
+
+
+if __name__ == "__main__":
+    unittest.main()