Tests and documentation

Also removes duplicate records before updating cache

Author: k1o0

CHANGELOG.md

@@ -41,7 +41,8 @@ Also adds a new ALFPath class to replace alf path functions and now returns UUID
 - 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
-- 'auto' cache mode has been removed
+- 'auto' and 'refresh' cache modes have been removed
+- One.refresh_cache
 
 ## [2.11.1]
 

docs/FAQ.md

@@ -28,13 +28,12 @@ context manager when calling ONE list, search and load methods. You can pass the
 argument AlyxClient: `one.alyx.rest(..., no_cache=True)`.  More information can be found
 [here](https://int-brain-lab.github.io/ONE/notebooks/one_modes.html#REST-caching).
 
-**Auto mode**
-Remote cache tables are downloaded used when ONE is in 'auto' mode (or when `query_type='auto'` is passed).
+**Local mode**
+Local cache tables may be used when ONE is in 'local' mode (or when `query_type='local'` is passed).
 These table contain info about all sessions and their associated datasets and is used instead of querying
 the database.
-For the IBL Alyx, the tables are generated every 6 hours, however by default ONE will only download a
-new cache once per day.  To force a download you can run `ONE().refresh_cache('remote')`.  More
-information, including increasing refresh frequency, can be found
+For the IBL Alyx, the tables are generated every 6 hours and can be downloaded using the `one.load_cache` method
+to only download when new data are available.  More information, including increasing refresh frequency, can be found
 [here](https://int-brain-lab.github.io/ONE/notebooks/one_modes.html#Refreshing-the-cache).
 
 ## I made a mistake during setup and now can't call setup, how do I fix it?
@@ -83,7 +82,7 @@ one = ONE(tables_dir=Path.home() / 'tables_dir')
 # 2. Specify location after instantiation
 one.load_cache(Path.home() / 'tables_dir')
 ```
-**Note**: Avoid using the same location for different database cache tables: by default ONE will automatically overwrite tables when a newer version is available. To avoid automatic downloading, set `mode='local'`.
+**Note**: Avoid using the same location for different database cache tables: ONE will overwrite tables when `load_cache` is called in remote mode.
 
 ## How do check who I'm logged in as?
 ```python

docs/notebooks/one_modes.ipynb

@@ -69,15 +69,18 @@
    "source": [
     "## Query modes\n",
     "In 'local' mode, the list, search and load methods will use the local cache tables and not\n",
-    "connect to Alyx, however the option to use Alyx is always there when a database is provided.  \n",
+    "connect to Alyx, however the option to use Alyx is always there when a database is provided.\n",
+    "When instantiating ONE in local mode, any cache tables on disk are loaded.\n",
     "\n",
     "If 'remote' mode is specified, ONE will only query the remote database and will not use the\n",
     "local cache tables.  Avoiding the database whenever possible is recommended as it doesn't rely\n",
     "on a stable internet connection and reduces the load on the remote database.\n",
     "\n",
     "While in 'remote' mode, the local cache may be used by providing the query_type='local' keyword\n",
-    "argument to any method.  Likewise, in 'local' mode, a remote query can be made by\n",
-    "specifying `query_type='remote'`\n",
+    "argument to any method.  This will then search based on the results of previous remote queries.\n",
+    "Likewise, in 'local' mode, a remote query can be made by specifying `query_type='remote'` (if a\n",
+    "database has been configured). The local cache tables will then be supplemented with the result\n",
+    "of this remote query. \n",
     "\n",
     "<div class=\"alert alert-info\">\n",
     "NB: The 'remote' query type is not valid in offline mode as there is no database associated to\n",
@@ -232,53 +235,6 @@
     "one = ONE(cache_rest=None, mode='remote')"
    ]
   },
-  {
-   "cell_type": "markdown",
-   "metadata": {
-    "collapsed": false,
-    "pycharm": {
-     "name": "#%% md\n"
-    }
-   },
-   "source": [
-    "## Refreshing the cache tables\n",
-    "While in 'auto' mode, ONE will try to update the cache tables once every 24 hours.\n",
-    "This can be changed by changing the 'cache_expiry' property:"
-   ]
-  },
-  {
-   "cell_type": "code",
-   "execution_count": 4,
-   "metadata": {
-    "collapsed": false,
-    "pycharm": {
-     "name": "#%%\n"
-    }
-   },
-   "outputs": [
-    {
-     "data": {
-      "text/plain": [
-       "{'expired': False,\n",
-       " 'created_time': datetime.datetime(2021, 9, 14, 13, 0),\n",
-       " 'loaded_time': datetime.datetime(2021, 9, 14, 18, 15, 54, 384591),\n",
-       " 'raw': {'datasets': {'date_created': '2021-09-14 13:00', 'origin': 'alyx'},\n",
-       "  'sessions': {'date_created': '2021-09-14 13:00', 'origin': 'alyx'}}}"
-      ]
-     },
-     "execution_count": 4,
-     "metadata": {},
-     "output_type": "execute_result"
-    }
-   ],
-   "source": [
-    "from datetime import timedelta\n",
-    "one.cache_expiry = timedelta(hours=1)  # Check for new remote cache every hour\n",
-    "\n",
-    "# The time when the cache was generated can be found in the cache metadata:\n",
-    "one._cache._meta"
-   ]
-  },
   {
    "cell_type": "markdown",
    "metadata": {
@@ -292,21 +248,6 @@
     "download.  The cache can be explicitly refreshed in two ways:"
    ]
   },
-  {
-   "cell_type": "code",
-   "execution_count": 5,
-   "metadata": {
-    "collapsed": false,
-    "pycharm": {
-     "name": "#%%\n"
-    }
-   },
-   "outputs": [],
-   "source": [
-    "loaded_time = one.refresh_cache('refresh')  # Explicitly refresh the cache\n",
-    "eids = one.search(lab='cortexlab', query_type='refresh')  # Calls `refresh_cache` before searching"
-   ]
-  },
   {
    "cell_type": "markdown",
    "metadata": {

one/api.py

@@ -87,7 +87,12 @@ def __init__(self, cache_dir=None, mode='local', wildcards=True, tables_dir=None
         self.uuid_filenames = False
         # init the cache file
         self._reset_cache()
-        self.load_cache()
+        if self.mode == 'local':
+            # Ensure that we don't call any subclass method here as we only load local cache
+            # tables on init. Direct calls to load_cache can be made by the user or subclass.
+            One.load_cache(self)
+        elif self.mode != 'remote':
+            raise ValueError(f'Mode "{self.mode}" not recognized')
 
     def __repr__(self):
         return f'One ({"off" if self.offline else "on"}line, {self.cache_dir})'
@@ -237,39 +242,6 @@ def _save_cache(self, save_dir=None, force=False):
                 _logger.debug(f'Saved {filename}')
             meta['saved_time'] = datetime.now()
 
-    def refresh_cache(self, mode=None):
-        """Check and reload cache tables.
-
-        Parameters
-        ----------
-        mode : {'local', 'refresh', 'remote'}
-            Options are 'local' (reload if expired); 'refresh' (reload); 'remote' (don't reload).
-
-        Returns
-        -------
-        datetime.datetime
-            Loaded timestamp.
-
-        """
-        # NB: Currently modified table will be lost if called with 'refresh';
-        # May be instances where modified cache is saved then immediately replaced with a new
-        # remote cache. Also it's too slow :(
-        # self.save_cache()  # Save cache if modified
-        mode = mode or self.mode
-        if mode == 'remote':
-            pass
-        elif mode == 'local':
-            loaded_time = self._cache['_meta']['loaded_time']
-            if not loaded_time or (datetime.now() - loaded_time >= self.cache_expiry):
-                _logger.info('Cache expired, refreshing')
-                self.load_cache()
-        elif mode == 'refresh':
-            _logger.debug('Forcing reload of cache')
-            self.load_cache(clobber=True)
-        else:
-            raise ValueError(f'Unknown refresh type "{mode}"')
-        return self._cache['_meta']['loaded_time']
-
     def _update_cache_from_records(self, strict=False, **kwargs):
         """Update the cache tables with new records.
 
@@ -309,6 +281,8 @@ def _update_cache_from_records(self, strict=False, **kwargs):
             if isinstance(records, pd.Series):
                 records = pd.DataFrame([records])
                 records.index.set_names(self._cache[table].index.names, inplace=True)
+            # Drop duplicate indices
+            records = records[~records.index.duplicated(keep='first')]
             if not strict:
                 # Deal with case where there are extra columns in the cache
                 extra_columns = list(set(self._cache[table].columns) - set(records.columns))
@@ -323,7 +297,7 @@ def _update_cache_from_records(self, strict=False, **kwargs):
                     records.insert(n, col, val)
                 # Drop any extra columns in the records that aren't in cache table
                 to_drop = set(records.columns) - set(self._cache[table].columns)
-                records.drop(to_drop, axis=1, inplace=True)
+                records = records.drop(to_drop, axis=1)
                 records = records.reindex(columns=self._cache[table].columns)
             assert set(self._cache[table].columns) == set(records.columns)
             records = records.astype(self._cache[table].dtypes)
@@ -1767,8 +1741,6 @@ def load_cache(self, tables_dir=None, clobber=False, tag=None):
         location and creation date of the remote cache.  If newer, it will be download and
         loaded.
 
-        Note: Unlike refresh_cache, this will always reload the local files at least once.
-
         Parameters
         ----------
         tables_dir : str, pathlib.Path
@@ -1802,7 +1774,7 @@ def load_cache(self, tables_dir=None, clobber=False, tag=None):
         if not (clobber or different_tag):
             super(OneAlyx, self).load_cache(tables_dir)  # Load any present cache
             expired = self._cache and (cache_meta := self._cache.get('_meta', {}))['expired']
-            if not expired or self.mode in {'local', 'remote'}:
+            if not expired:
                 return
 
         # Warn user if expired
@@ -2598,9 +2570,10 @@ def _dset2url(self, dset, update_cache=True):
         if isinstance(dset, str) and dset.startswith('http'):
             url = dset
         elif isinstance(dset, (str, Path)):
-            url = self.path2url(dset)
-            if not url:
-                _logger.warning(f'Dataset {dset} not found in cache')
+            try:
+                url = self.path2url(dset)
+            except alferr.ALFObjectNotFound:
+                _logger.warning(f'Dataset {dset} not found')
                 return
         elif isinstance(dset, (list, tuple)):
             dset2url = partial(self._dset2url, update_cache=update_cache)
@@ -2914,7 +2887,12 @@ def path2url(self, filepath, query_type=None) -> str:
             return super(OneAlyx, self).path2url(filepath)
         eid = self.path2eid(filepath)
         try:
-            dataset, = self.alyx.rest('datasets', 'list', session=eid, name=Path(filepath).name)
+            params = {'name': Path(filepath).name}
+            if eid is None:
+                params['django'] = 'session__isnull,True'
+            else:
+                params['session'] = str(eid)
+            dataset, = self.alyx.rest('datasets', 'list', **params)
             return next(
                 r['data_url'] for r in dataset['file_records'] if r['data_url'] and r['exists'])
         except (ValueError, StopIteration):

one/tests/test_converters.py

@@ -243,6 +243,7 @@ class TestOnlineConverters(unittest.TestCase):
     def setUpClass(cls) -> None:
         # Create ONE object with temp cache dir
         cls.one = ONE(**TEST_DB_2)
+        cls.one.load_cache()  # load local cache tables
         cls.eid = UUID('4ecb5d24-f5cc-402c-be28-9d0f7cb14b3a')
         cls.pid = UUID('da8dfec1-d265-44e8-84ce-6ae9c109b8bd')
         cls.session_record = cls.one.get_details(cls.eid)