Remove deprecated

CHANGELOG.md

@@ -15,6 +15,7 @@ Also adds a new ALFPath class to replace alf path functions.
 - one.alf.path module containing ALFPath class
 - ALF cache table generation has lower memory footprint
 - setup in silent mode now uses defaults if base url matches default one
+- bugfix: error downloading from http server with keep_uuids=True
 
 ### Added
 
@@ -29,6 +30,9 @@ Also adds a new ALFPath class to replace alf path functions.
 - setup.py
 - one.alf.files; use one.alf.path instead
 - one.alf.io.remove_uuid_file
+- one.alf.io.remove_uuid_recursive
+- one.util.ensure_list; use iblutil.util.ensure_list instead
+- one.remote.globus.create_globus_client; use one.remote.globus.Globus class instead
 
 ## [2.11.1]
 

one/alf/io.py

@@ -666,27 +666,6 @@ def save_metadata(file_alf, dico) -> path.ALFPath:
     return file_meta_data
 
 
-def remove_uuid_recursive(folder, dry=False) -> None:
-    """
-    (DEPRECATED) Within a folder, recursive renaming of all files to remove UUID.
-
-    Parameters
-    ----------
-    folder : str, pathlib.Path
-        A folder to recursively iterate, removing UUIDs from the file names.
-    dry : bool
-        If False renames the files on disk.
-    """
-    warnings.warn(
-        'remove_uuid_recursive is deprecated and will be removed in the next release',
-        DeprecationWarning)
-    for fn in path.ALFPath(folder).iter_datasets(recursive=True):
-        if (new_fn := fn.without_uuid()).name != fn.name:
-            print(new_fn)
-            if not dry:
-                fn.rename(new_fn)
-
-
 def next_num_folder(session_date_folder: Union[str, Path]) -> str:
     """Return the next number for a session given a session_date_folder."""
     session_date_folder = Path(session_date_folder)

one/api.py

@@ -1607,12 +1607,6 @@ def ONE(*, mode='remote', wildcards=True, **kwargs):
     One, OneAlyx
         An One instance if mode is 'local', otherwise an OneAlyx instance.
     """
-    if kwargs.pop('offline', False):
-        _logger.warning('the offline kwarg will probably be removed. '
-                        'ONE is now offline by default anyway')
-        warnings.warn('"offline" param will be removed; use mode="local"', FutureWarning)
-        mode = 'local'
-
     if (any(x in kwargs for x in ('base_url', 'username', 'password')) or
             not kwargs.get('cache_dir', False)):
         return OneAlyx(mode=mode, wildcards=wildcards, **kwargs)
@@ -2528,7 +2522,8 @@ def _download_dataset(
             return
         if isinstance(url, str):
             target_dir = str(Path(cache_dir, alfiles.get_alf_path(url)).parent)
-            return self._download_file(url, target_dir, **kwargs)
+            file = self._download_file(url, target_dir, **kwargs)
+            return ALFPath(file) if file else None
         # must be list of URLs
         valid_urls = list(filter(None, url))
         if not valid_urls:
@@ -2564,6 +2559,7 @@ def _tag_mismatched_file_record(self, url):
     def _download_file(self, url, target_dir, keep_uuid=None, file_size=None, hash=None):
         """
         Downloads a single file or multitude of files from an HTTP webserver.
+
         The webserver in question is set by the AlyxClient object.
 
         Parameters
@@ -2581,7 +2577,7 @@ def _download_file(self, url, target_dir, keep_uuid=None, file_size=None, hash=N
 
         Returns
         -------
-        one.alf.path.ALFPath or list of one.alf.path.ALFPath
+        pathlib.Path or list of pathlib.Path
             The file path of the downloaded file or files.
 
         Example
@@ -2605,7 +2601,7 @@ def _download_file(self, url, target_dir, keep_uuid=None, file_size=None, hash=N
 
         # check if we are keeping the uuid on the list of file names
         if keep_uuid is True or (keep_uuid is None and self.uuid_filenames):
-            return local_path
+            return list(local_path) if isinstance(local_path, tuple) else local_path
 
         # remove uuids from list of file names
         if isinstance(local_path, (list, tuple)):

one/remote/globus.py

@@ -75,7 +75,7 @@
 >>> file = glo.download_file('path/to/file.ext', 'source_endpoint')
 Path('/path/to/new/location/path/to/file.ext')
 
-Await multiple tasks to complete by passing a list of Globus tranfer IDs
+Await multiple tasks to complete by passing a list of Globus transfer IDs
 
 >>> import asyncio
 >>> tasks = [asyncio.create_task(globus.task_wait_async(task_id))) for task_id in task_ids]
@@ -249,45 +249,6 @@ def get_token(client_id, refresh_tokens=True):
         return dict.fromkeys(fields)
 
 
-def create_globus_client(client_name='default'):
-    """
-    Creates a Globus transfer client based on existing parameter file.
-
-    Parameters
-    ----------
-    client_name : str
-        Defines the parameter name to use (globus.client_name), e.g. 'default', 'admin'.
-
-    Returns
-    -------
-    globus_sdk.TransferClient
-        Globus transfer client instance.
-    """
-    msg = 'create_globus_client will be removed in the future, use Globus class instead.'
-    warnings.warn(msg, FutureWarning)
-    try:
-        globus_pars = load_client_params(f'{CLIENT_KEY}.{client_name}')
-    except (AttributeError, FileNotFoundError):
-        _setup(client_name, login=True, refresh_tokens=True)
-        globus_pars = load_client_params(f'{CLIENT_KEY}.{client_name}', assert_present=False) or {}
-
-    # Assert valid Globus client ID found
-    if not (globus_pars and 'GLOBUS_CLIENT_ID' in iopar.as_dict(globus_pars)
-            and is_uuid(globus_pars.GLOBUS_CLIENT_ID)):
-        raise ValueError(
-            'Invalid Globus client ID in client parameter file. Rerun Globus.setup'
-        )
-
-    if iopar.as_dict(globus_pars).get('refresh_token'):
-        client = globus_sdk.NativeAppAuthClient(globus_pars.GLOBUS_CLIENT_ID)
-        client.oauth2_start_flow(refresh_tokens=True)
-        authorizer = globus_sdk.RefreshTokenAuthorizer(globus_pars.refresh_token, client)
-        return globus_sdk.TransferClient(authorizer=authorizer)
-    else:
-        authorizer = globus_sdk.AccessTokenAuthorizer(globus_pars.access_token)
-        return globus_sdk.TransferClient(authorizer=authorizer)
-
-
 def _remove_token_fields(pars):
     """
     Remove the token fields from a parameters object.

one/tests/alf/test_alf_io.py

@@ -594,25 +594,6 @@ def test_load_sparse_npz(self):
             self.assertEqual(loaded, self.sparse_npz)
 
 
-class TestUUID_Files(unittest.TestCase):
-
-    def test_remove_uuid_recusive(self):
-        uuid = '30c09473-4d3d-4f51-9910-c89a6840096e'
-        with tempfile.TemporaryDirectory() as dir:
-            f1 = Path(dir).joinpath(f'tutu.part1.part1.{uuid}.json')
-            f2 = Path(dir).joinpath('tata.part1.part1.json')
-            f3 = Path(dir).joinpath('toto.json')
-            f4 = Path(dir).joinpath('collection', f'tutu.part1.part1.{uuid}.json')
-            f1.touch()
-            f2.touch()
-            f2.touch()
-            f3.touch()
-            f4.parent.mkdir()
-            f4.touch()
-            alfio.remove_uuid_recursive(Path(dir))
-            self.assertFalse(len(list(Path(dir).rglob(f'*{uuid}*'))))
-
-
 class TestALFFolders(unittest.TestCase):
     tempdir = None
     session_path = None

one/tests/remote/test_globus.py

@@ -179,38 +179,6 @@ def test_get_lab_from_endpoint_id(self):
             name = globus.get_lab_from_endpoint_id(alyx=ac)[0]
             self.assertEqual(name, 'mainenlab')
 
-    @mock.patch('one.remote.globus.globus_sdk')
-    def test_create_globus_client(self, globus_mock):
-        """Tests for one.remote.globus.create_globus_client function."""
-        # Check setup run when no params exist, check raises exception when missing params
-        gc_id = str(uuid.uuid4())
-        incomplete_pars = iopar.from_dict({'GLOBUS_CLIENT_ID': gc_id})
-        with mock.patch('one.remote.globus._setup') as setup_mock, \
-             self.assertRaises(ValueError), \
-             mock.patch('one.remote.base.load_client_params',
-                        side_effect=[AssertionError, incomplete_pars]):
-            globus.create_globus_client()
-            setup_mock.assert_called()
-
-        # Check behaviour with complete params
-        pars = iopar.from_dict({'GLOBUS_CLIENT_ID': gc_id, 'refresh_token': 456})
-        with mock.patch('one.remote.globus.load_client_params', return_value=pars) as par_mock:
-            client = globus.create_globus_client('admin')
-            par_mock.assert_called_once_with('globus.admin')
-        globus_mock.NativeAppAuthClient.assert_called_once_with(gc_id)
-        globus_mock.RefreshTokenAuthorizer.assert_called()
-        self.assertEqual(client, globus_mock.TransferClient())
-
-        # Check without refresh tokens
-        pars = pars.set('refresh_token', None).set('access_token', 456)
-        globus_mock.RefreshTokenAuthorizer.reset_mock()
-        with mock.patch('one.remote.globus.load_client_params', return_value=pars) as par_mock:
-            client = globus.create_globus_client('admin')
-            par_mock.assert_called_once_with('globus.admin')
-        globus_mock.AccessTokenAuthorizer.assert_called_once_with(456)
-        globus_mock.RefreshTokenAuthorizer.assert_not_called()
-        self.assertEqual(client, globus_mock.TransferClient())
-
     def test_remove_token_fields(self):
         """Test for one.remote.globus._remove_token_fields function."""
         par = iopar.from_dict({
@@ -306,7 +274,6 @@ def test_setup(self):
         TestGlobus.test_setup tests the setup function. Here we just check it's called.
         """
         with mock.patch('one.remote.globus._setup') as setup_mock, \
-                mock.patch('one.remote.globus.create_globus_client'), \
                 mock.patch('one.remote.globus.load_client_params', return_value=self.pars):
             self.assertIsInstance(globus.Globus.setup(), globus.Globus)
             setup_mock.assert_called_once()

one/tests/test_one.py

@@ -48,7 +48,7 @@
 from one.api import ONE, One, OneAlyx
 from one.util import (
     validate_date_range, index_last_before, filter_datasets, _collection_spec,
-    filter_revision_last_before, parse_id, autocomplete, LazyId, ensure_list
+    filter_revision_last_before, parse_id, autocomplete, LazyId
 )
 import one.params
 import one.alf.exceptions as alferr
@@ -2036,10 +2036,6 @@ def test_one_factory(self):
             one_obj = ONE(cache_dir=self.tempdir.name)
             self.assertIsInstance(one_obj, One)
 
-            # The offline param was given, raise deprecation warning (via log)
-            with self.assertWarns(FutureWarning):
-                ONE(offline=True, cache_dir=self.tempdir.name)
-
             # Test setup with virtual ONE method
             assert ONE.cache_info().currsize > 0
             ONE.setup(silent=True, make_default=True)
@@ -2214,11 +2210,6 @@ def test_LazyID(self):
         ez = LazyId([{'id': x} for x in uuids])
         self.assertCountEqual(map(str, ez), uuids)
 
-    def test_ensure_list(self):
-        """Test one.util.ensure_list.
 
-        This function has now moved therefore we simply check for deprecation warning.
-        """
-        with self.assertWarns(DeprecationWarning):
-            self.assertEqual(['123'], ensure_list('123'))
-            self.assertIs(x := ['123'], ensure_list(x))
+if __name__ == '__main__':
+    unittest.main(exit=True, verbosity=2)

one/util.py

@@ -8,8 +8,8 @@
 from collections.abc import Mapping
 
 import pandas as pd
-from iblutil.util import ensure_list as _ensure_list
 import numpy as np
+from iblutil.util import ensure_list
 
 import one.alf.exceptions as alferr
 from one.alf.path import rel_path_parts
@@ -291,7 +291,7 @@ def filter_datasets(
         exclude_slash = partial(re.sub, fr'^({flagless_token})?\.\*', r'\g<1>[^/]*')
         spec_str += '|'.join(
             exclude_slash(fnmatch.translate(x)) if wildcards else x + '$'
-            for x in _ensure_list(filename)
+            for x in ensure_list(filename)
         )
 
     # If matching revision name, add to regex string
@@ -303,7 +303,7 @@ def filter_datasets(
             continue
         if wildcards:
             # Convert to regex, remove \\Z which asserts end of string
-            v = (fnmatch.translate(x).replace('\\Z', '') for x in _ensure_list(v))
+            v = (fnmatch.translate(x).replace('\\Z', '') for x in ensure_list(v))
         if not isinstance(v, str):
             regex_args[k] = '|'.join(v)  # logical OR
 
@@ -485,14 +485,6 @@ def autocomplete(term, search_terms) -> str:
     return key_
 
 
-def ensure_list(value):
-    """Ensure input is a list."""
-    warnings.warn(
-        'one.util.ensure_list is deprecated, use iblutil.util.ensure_list instead',
-        DeprecationWarning)
-    return _ensure_list(value)
-
-
 class LazyId(Mapping):
     """
     Using a paginated response object or list of session records, extracts eid string when required

one/webclient.py

@@ -820,7 +820,8 @@ def download_file(self, url, **kwargs):
 
         Returns
         -------
-        Local path(s) of downloaded file(s).
+        pathlib.Path, list of pathlib.Path
+            Local path(s) of downloaded file(s).
         """
         if isinstance(url, str):
             url = self._validate_file_url(url)