Repository: tensorflow/datasets (python)

Issue:
Progressbar Not working TFDS FTP Downloader
**Download Progressbar Not working for downloads over ftp**
Download Progressbar Not working for Downloads over FTP

**Environment information**
* Operating System: Debian 4.9.130-2
* Python version: Python 3
* `tensorflow-datasets`/`tfds-nightly` version: 1.0.1
* `tensorflow`/`tensorflow-gpu`/`tf-nightly`/`tf-nightly-gpu` version: 1.13.1

**Reproduction instructions**

```
import tensorflow_datasets as tfds
dm = tfds.download.DownloadManager(download_dir=".")
path = dm.download_and_extract("ftp://cs.stanford.edu/cs/cvgl/Stanford_Online_Products.zip")
```
**Expected behavior**
The Download ProgressBar is expected to fill while downloading the Dataset.

**Additional context**
The Download is Successful however, The DL: ProgressBar is Not Filling Up.


Pull request:
Fixed FTP Downloader
Fixed #231. @cyfra take a look

Patch:
diff --git a/tensorflow_datasets/core/download/downloader.py b/tensorflow_datasets/core/download/downloader.py
index 9dd5a58e6..8a6b6af97 100644
--- a/tensorflow_datasets/core/download/downloader.py
+++ b/tensorflow_datasets/core/download/downloader.py
@@ -142,21 +142,12 @@ class _Downloader(object):
         out_path, checksum_cls=self._checksumer)
     return hexdigest, size
 
-  def _sync_ftp_download(self, url, destination_path):
-    out_path = os.path.join(destination_path, download_util.get_file_name(url))
-    urllib.request.urlretrieve(url, out_path)
-    hexdigest, size = utils.read_checksum_digest(
-        out_path, checksum_cls=self._checksumer)
-    return hexdigest, size
 
   def _sync_download(self, url, destination_path):
     """Synchronous version of `download` method."""
     if kaggle.KaggleFile.is_kaggle_url(url):
       return self._sync_kaggle_download(url, destination_path)
 
-    if url.startswith('ftp'):
-      return self._sync_ftp_download(url, destination_path)
-
     try:
       # If url is on a filesystem that gfile understands, use copy. Otherwise,
       # use requests.
@@ -168,10 +159,15 @@ class _Downloader(object):
     session = requests.Session()
     if _DRIVE_URL.match(url):
       url = self._get_drive_url(url, session)
-    response = session.get(url, stream=True)
-    if response.status_code != 200:
-      raise DownloadError(
-          'Failed to get url %s. HTTP code: %d.' % (url, response.status_code))
+    if not url.startswith('ftp'):
+      response = session.get(url, stream=True)
+      if response.status_code != 200:
+        raise DownloadError(
+          'Failed to get url %s. HTTP code: %d.' %
+          (url, response.status_code))
+    else:
+      request = urllib.request.Request(url)
+      response = urllib.request.urlopen(request)
     fname = _get_filename(response)
     path = os.path.join(destination_path, fname)
     size = 0
@@ -182,7 +178,12 @@ class _Downloader(object):
         int(response.headers.get('Content-length', 0)) // unit_mb)
     with tf.io.gfile.GFile(path, 'wb') as file_:
       checksum = self._checksumer()
-      for block in response.iter_content(chunk_size=io.DEFAULT_BUFFER_SIZE):
+      if not url.startswith("ftp"):
+        iterator = response.iter_content(chunk_size=io.DEFAULT_BUFFER_SIZE)
+      else:
+        iterator = iter(lambda: response.read(io.DEFAULT_BUFFER_SIZE), b'')
+
+      for block in iterator:
         size += len(block)
 
         # Update the progress bar


Test patch:
diff --git a/tensorflow_datasets/core/download/downloader_test.py b/tensorflow_datasets/core/download/downloader_test.py
index 0ae8c4fed..3e45c670e 100644
--- a/tensorflow_datasets/core/download/downloader_test.py
+++ b/tensorflow_datasets/core/download/downloader_test.py
@@ -39,6 +39,7 @@ class _FakeResponse(object):
     self.cookies = cookies or {}
     self.headers = headers or {'Content-length': 12345}
     self.status_code = status_code
+    self.read = self.raw.read
 
   def iter_content(self, chunk_size):
     del chunk_size
@@ -67,14 +68,10 @@ class DownloaderTest(testing.TestCase):
     self.downloader._pbar_url = absltest.mock.MagicMock()
     self.downloader._pbar_dl_size = absltest.mock.MagicMock()
 
-    def write_fake_ftp_result(_, filename):
-      with open(filename, 'wb') as result:
-        result.write(self.response)
-
     absltest.mock.patch.object(
-        downloader.urllib.request,
-        'urlretrieve',
-        write_fake_ftp_result,
+      downloader.urllib.request,
+      'urlopen',
+      lambda *a, **kw: _FakeResponse(self.url, self.response, self.cookies),
     ).start()
 
   def test_ok(self):
@@ -141,7 +138,7 @@ class DownloaderTest(testing.TestCase):
     error = downloader.urllib.error.URLError('Problem serving file.')
     absltest.mock.patch.object(
         downloader.urllib.request,
-        'urlretrieve',
+        'urlopen',
         side_effect=error,
     ).start()
     resource = resource_lib.Resource(url='ftp://example.com/foo.tar.gz')
diff --git a/tensorflow_datasets/testing/horses_or_humans.py b/tensorflow_datasets/testing/horses_or_humans.py
index 9e0e37db1..819357a8a 100644
--- a/tensorflow_datasets/testing/horses_or_humans.py
+++ b/tensorflow_datasets/testing/horses_or_humans.py
@@ -13,9 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-r"""Generate fake data for rock_paper_scissors dataset.
-
-"""
+r"""Generate fake data for horses_or_humans dataset."""
 
 from __future__ import absolute_import
 from __future__ import division