Prompt user to delete old table files (issue #194)

Author: k1o0

CHANGELOG.md

@@ -27,6 +27,7 @@ Also adds a new ALFPath class to replace alf path functions and now returns UUID
 - One.save_cache now updates any tables on disk unless clobber is True
 - one.util.patch_cache -> one.alf.cache.patch_tables
 - One object will save modified tables to disk upon delete
+- bugfix: ONE_HTTP_DL_THREADS environment variable now works as intended
 
 ### Added
 

one/__init__.py

@@ -1,2 +1,2 @@
 """The Open Neurophysiology Environment (ONE) API."""
-__version__ = '3.0b4'
+__version__ = '3.0b5'

one/api.py

@@ -165,6 +165,25 @@ def load_cache(self, tables_dir=None, **kwargs):
             if self.offline:  # In online mode, the cache tables should be downloaded later
                 warnings.warn(f'No cache tables found in {self._tables_dir}')
 
+        # If in remote mode and loading old tables generated on Alyx,
+        # prompt the user to delete them to improve load times
+        raw_meta = self._cache['_meta'].get('raw', {}).values() or [{}]
+        tagged = any(filter(None, flatten(x.get('database_tags') for x in raw_meta)))
+        origin = set(x['origin'] for x in raw_meta if 'origin' in x)
+        older = (self._cache['_meta']['created_time'] or datetime.now()) < datetime(2025, 2, 13)
+        remote = not self.offline and self.mode == 'remote'
+        if remote and origin == {'alyx'} and older and not self._web_client.silent and not tagged:
+            message = ('Old Alyx cache tables detected on disk. '
+                      'It\'s recomended to remove these tables as they '
+                      'negatively affect performance.\nDelete these tables? [Y/n]: ')
+            if (input(message).casefold().strip() or 'y')[0] == 'y':
+                self._remove_table_files()
+                self._reset_cache()
+        elif len(self._cache.datasets) > 1e6:
+            warnings.warn(
+                'Large cache tables affect performance. '
+                'Consider removing them by calling the `_remove_table_files` method.')
+
         return self._cache['_meta']['loaded_time']
 
     def save_cache(self, save_dir=None, clobber=False):
@@ -1646,6 +1665,11 @@ def load_cache(self, tables_dir=None, clobber=False, tag=None):
         tag : str
             An optional Alyx dataset tag for loading cache tables containing a subset of datasets.
 
+        Returns
+        -------
+        datetime.datetime
+            A timestamp of when the cache was loaded.
+
         Examples
         --------
         To load the cache tables for a given release tag
@@ -1669,6 +1693,8 @@ def load_cache(self, tables_dir=None, clobber=False, tag=None):
         different_tag = any(x != tag for x in current_tags)
         if not (clobber or different_tag):
             super(OneAlyx, self).load_cache(tables_dir)  # Load any present cache
+            cache_meta = self._cache.get('_meta', {})
+            raw_meta = cache_meta.get('raw', {}).values() or [{}]
 
         try:
             # Determine whether a newer cache is available
@@ -1679,15 +1705,15 @@ def load_cache(self, tables_dir=None, clobber=False, tag=None):
             min_version = packaging.version.parse(cache_info.get('min_api_version', '0.0.0'))
             if packaging.version.parse(one.__version__) < min_version:
                 warnings.warn(f'Newer cache tables require ONE version {min_version} or greater')
-                return
+                return cache_meta['loaded_time']
 
             # Check whether remote cache more recent
             remote_created = datetime.fromisoformat(cache_info['date_created'])
             local_created = cache_meta.get('created_time', None)
             fresh = local_created and (remote_created - local_created) < timedelta(minutes=1)
             if fresh and not different_tag:
                 _logger.info('No newer cache available')
-                return
+                return cache_meta['loaded_time']
 
             # Set the cache table directory location
             if tables_dir:  # If tables directory specified, use that
@@ -1711,7 +1737,7 @@ def load_cache(self, tables_dir=None, clobber=False, tag=None):
             _logger.info('Downloading remote caches...')
             files = self.alyx.download_cache_tables(cache_info.get('location'), self._tables_dir)
             assert any(files)
-            super(OneAlyx, self).load_cache(self._tables_dir)  # Reload cache after download
+            return super(OneAlyx, self).load_cache(self._tables_dir)  # Reload cache after download
         except (requests.exceptions.HTTPError, wc.HTTPError, requests.exceptions.SSLError) as ex:
             _logger.debug(ex)
             _logger.error(f'{type(ex).__name__}: Failed to load the remote cache file')
@@ -1728,6 +1754,7 @@ def load_cache(self, tables_dir=None, clobber=False, tag=None):
                 'Please provide valid tables_dir / cache_dir kwargs '
                 'or run ONE.setup to update the default directory.'
             )
+        return cache_meta['loaded_time']
 
     @property
     def alyx(self):

one/tests/test_one.py

@@ -955,6 +955,40 @@ def test_load_cache(self):
             with self.assertRaises(KeyError):
                 self.one.load_cache(tdir)
 
+        # Test loading large Alyx tables
+        raw = {'origin': 'alyx'}
+        cache = Bunch({
+            'datasets': EMPTY_DATASETS_FRAME.copy(),
+            'sessions': EMPTY_SESSIONS_FRAME.copy(),
+            '_meta': {
+                'created_time': datetime.datetime(2025, 2, 1, 12, 0),
+                'loaded_time': datetime.datetime.now(),
+                'raw': {'datasets': raw, 'sessions': raw}}
+        })
+        with mock.patch('one.api.load_tables', return_value=cache), \
+                mock.patch('builtins.input', return_value='yes'), \
+                mock.patch.object(self.one, '_remove_table_files') as m:
+            self.one.load_cache()
+            m.assert_not_called()
+        # Remote mode
+        self.one.mode = 'remote'
+        self.one._web_client = mock.MagicMock()
+        self.one._web_client.silent = False
+        with mock.patch('one.api.load_tables', return_value=cache), \
+                mock.patch('builtins.input', return_value='yes'), \
+                mock.patch.object(self.one, '_remove_table_files') as m:
+            self.one.load_cache()
+            m.assert_called_once()
+        # Test large table warning
+        cache.datasets = mock.MagicMock()
+        cache.datasets.__len__.return_value = int(1.5e6)
+        self.one._web_client = None
+        self.one.mode = 'local'
+        with mock.patch('one.api.load_tables', return_value=cache), \
+                mock.patch('builtins.input', return_value='n'), \
+                mock.patch.object(self.one, '_remove_table_files') as m:
+            self.assertWarns(UserWarning, self.one.load_cache)
+
     def test_save_cache(self):
         """Test One.save_cache method."""
         self.one._cache['_meta'].pop('modified_time', None)
@@ -1311,7 +1345,8 @@ def test_load_cache(self):
                     mock.patch.object(self.one.alyx, 'get', return_value=cache_info), \
                     mock.patch('one.api.One.load_cache', side_effect=now), \
                     self.assertLogs('one.api', 'INFO') as lg:
-                self.assertIsNone(self.one.load_cache(tag='Q3-2020-TAG'))
+                expected = self.one._cache['_meta']['loaded_time']
+                self.assertEqual(expected, self.one.load_cache(tag='Q3-2020-TAG'))
             cm.assert_not_called()
             self.assertRegex(lg.output[-1], 'No newer cache available')