account for public being in rel_path (#127)

* account for public being in rel_path
* Slightly more generalized version

---------

Co-authored-by: Miles Wells <[email protected]>

CHANGELOG.md

@@ -5,6 +5,8 @@
 
 - HOTFIX: When downloading cache only authenticate Alyx when necessary
 - HOTFIX: Ensure http data server URL does not end in slash
+- HOTFIX: Handle public aggregate dataset relative paths
+- HOTFIX: No longer warns in silent mode when no param conflicts present
 - Explicit kwargs in load_* methods to avoid user confusion (e.g. no 'namespace' kwarg for `load_dataset`)
 
 ## [2.9.0]

one/api.py

@@ -1827,13 +1827,13 @@ def list_aggregates(self, relation: str, identifier: str = None,
                    .reset_index(level=0)
                    .drop('eid', axis=1)
                    .rename_axis(index={'id': 'did'}))
-        # Since rel_path for public FI file records starts with public/aggregates instead of just
-        # aggregates as is the case for internal FI file records, as well as public and internal
-        # AWS file records, we need to make sure to use the file path part after 'aggregates'
-        # and not simply the second part, as relation.
+        # Since rel_path for public FI file records starts with 'public/aggregates' instead of just
+        # 'aggregates', we should discard the file path parts before 'aggregates' (if present)
+        records['rel_path'] = records['rel_path'].str.replace(
+            r'^[\w\/]+(?=aggregates\/)', '', n=1, regex=True)
+        # The relation is the first part after 'aggregates', i.e. the second part
         records['relation'] = records['rel_path'].map(
-            lambda x: x.split('aggregates')[-1].split('/')[1].lower()
-        )
+            lambda x: x.split('aggregates')[-1].split('/')[1].lower())
         records = records[records['relation'] == relation.lower()]
 
         def path2id(p) -> str:

one/params.py

@@ -182,12 +182,12 @@ def setup(client=None, silent=False, make_default=None, username=None, cache_dir
             print('SETUP ABANDONED.  Please re-run.')
             return par_current
     else:
-        par = par_current
-        if any(v for k, v in cache_map.CLIENT_MAP.items() if k != client_key):
-            warnings.warn('Warning: the directory provided is already a cache for another URL.')
         # Precedence: user provided cache_dir; previously defined; the default location
         default_cache_dir = Path(CACHE_DIR_DEFAULT, client_key)
         cache_dir = cache_dir or cache_map.CLIENT_MAP.get(client_key, default_cache_dir)
+        par = par_current
+        if any(v for k, v in cache_map.CLIENT_MAP.items() if k != client_key and v == cache_dir):
+            warnings.warn('Warning: the directory provided is already a cache for another URL.')
 
     # Update and save parameters
     Path(cache_dir).mkdir(exist_ok=True, parents=True)

one/tests/test_one.py

@@ -1249,14 +1249,15 @@ def test_list_aggregates(self):
         """Test OneAlyx.list_aggregates"""
         # Test listing by relation
         datasets = self.one.list_aggregates('subjects')
-        self.assertTrue(all(datasets['rel_path'].str.startswith('cortexlab/Subjects')))
+        self.assertTrue(all(datasets['rel_path'].str.startswith('aggregates/Subjects')))
         self.assertIn('exists_aws', datasets.columns)
         self.assertIn('session_path', datasets.columns)
         self.assertTrue(all(datasets['session_path'] == ''))
         self.assertTrue(self.one.list_aggregates('foobar').empty)
         # Test filtering with an identifier
         datasets = self.one.list_aggregates('subjects', 'ZM_1085')
-        self.assertTrue(all(datasets['rel_path'].str.startswith('cortexlab/Subjects/ZM_1085')))
+        expected = 'aggregates/Subjects/mainenlab/ZM_1085'
+        self.assertTrue(all(datasets['rel_path'].str.startswith(expected)))
         self.assertTrue(self.one.list_aggregates('subjects', 'foobar').empty)
         # Test that additional parts of data path are correctly removed
         # specifically /public in FI openalyx file rec

one/tests/test_params.py

@@ -61,7 +61,7 @@ def test_setup(self, _):
             self.assertNotEqual(cache.ALYX_LOGIN, 'mistake')
 
         # Check that raises ValueError when bad URL provided
-        self.url = 'ftp://'
+        self.url = 'ftp://foo.bar.org'
         with self.assertRaises(ValueError), mock.patch('one.params.input', new=self._mock_input):
             one.params.setup()