Merge pull request #406 from MLMI2-CSSI/fix_main

remove breaking changes that load on init

Author: blue442

foundry/foundry.py

@@ -50,27 +50,17 @@ class Foundry(FoundryBase):
     auths: Any
 
     def __init__(
-            self, name=None, no_browser=False, no_local_server=False, index="mdf", authorizers=None,
-            download=True, globus=True, verbose=False, metadata=None, interval=10,
-            **data
+            self, no_browser=False, no_local_server=False, index="mdf", authorizers=None, **data
     ):
         """Initialize a Foundry client
         Args:
-            name (str): Name of the foundry dataset. If not supplied, metadata will not be loaded into
-                    the Foundry object
             no_browser (bool):  Whether to open the browser for the Globus Auth URL.
             no_local_server (bool): Whether a local server is available.
                     This should be `False` when on remote server (e.g., Google Colab ).
             index (str): Index to use for search and data publication. Choices `mdf` or `mdf-test`
             authorizers (dict): A dictionary of authorizers to use, following the `mdf_toolbox` format
-            download (bool): If True, download the data associated with the package (default is True)
-            globus (bool): If True, download using Globus, otherwise https
-            verbose (bool): If True print additional debug information
-            metadata (dict): **For debug purposes.** A search result analog to prepopulate metadata.
-            interval (int): How often to poll Globus to check if transfers are complete
             data (dict): Other arguments, e.g., results from an MDF search result that are used
                     to populate Foundry metadata fields
-
         Returns:
             an initialized and authenticated Foundry client
         """
@@ -155,29 +145,20 @@ def __init__(
             force_login=False,
         )
 
-        if name is not None:
-            self._load(name=name,
-                       download=download,
-                       globus=globus,
-                       verbose=verbose,
-                       metadata=metadata,
-                       authorizers=authorizers,
-                       interval=interval)
-
-    def _load(self, name, download=True, globus=True, verbose=False, metadata=None, authorizers=None, interval=None):
+    def load(self, name, download=True, globus=False, verbose=False, metadata=None, authorizers=None, **kwargs):
         """Load the metadata for a Foundry dataset into the client
         Args:
             name (str): Name of the foundry dataset
             download (bool): If True, download the data associated with the package (default is True)
             globus (bool): If True, download using Globus, otherwise https
             verbose (bool): If True print additional debug information
             metadata (dict): **For debug purposes.** A search result analog to prepopulate metadata.
+        Keyword Args: (TODO: make this a regular arg instead?)
             interval (int): How often to poll Globus to check if transfers are complete
 
         Returns:
             self
         """
-
         # handle empty dataset name (was returning all the datasets)
         if not name:
             raise ValueError("load: No dataset name is given")
@@ -220,8 +201,9 @@ def _load(self, name, download=True, globus=True, verbose=False, metadata=None,
         self.dataset = FoundryDataset(**res['dataset'])
 
         if download:  # Add check for package existence
+            # TODO: perhaps change interval here
             self.download(
-                interval=interval, globus=globus, verbose=verbose
+                interval=kwargs.get("interval", 10), globus=globus, verbose=verbose
             )
 
         return self
@@ -425,6 +407,8 @@ def publish_dataset(
         self.connect_client.set_project_block(
             self.config.metadata_key, foundry_metadata)
 
+        # NOTE: need to add data_source and set_block before submit_dataset() is called -- so might want to upload
+        #   from here and not from within submit_dataset()?
         # upload via HTTPS if specified
         if https_data_path:
             # gather auth'd clients necessary for publication to endpoint
@@ -437,6 +421,20 @@ def publish_dataset(
             )
             # upload (ie publish) data to endpoint
             globus_data_source = upload_to_endpoint(pub_auths, https_data_path, endpoint_id)
+
+        """
+        # proposed solution
+        .
+        .
+        self.connect_client.set_project_block()
+        # auths handled within MDF? attempt this
+        # consider renaming to "https_upload" or similar
+        if https_data_path:
+            globus_data_source = self.connect_client.upload_to_endpoint(https_data_path) #leave endpoint_id as an option
+        self.connect_client.add_data_source(globus_data_source)
+        .
+        .
+        """
         # set Globus data source URL with MDF
         self.connect_client.add_data_source(globus_data_source)
         # set dataset name using the title if an abbreviated short_name isn't specified

tests/test_foundry.py

@@ -207,33 +207,33 @@ def _delete_test_data(foundry_obj):
 
 
 def test_foundry_init():
-    f = Foundry(test_dataset, download=False, authorizers=auths)
+    f = Foundry(authorizers=auths)
     assert isinstance(f.forge_client, Forge)
     assert isinstance(f.connect_client, MDFConnectClient)
 
     if not is_gha:
         assert isinstance(f.dlhub_client, DLHubClient)
 
-        f2 = Foundry(test_dataset, download=False, authorizers=auths, no_browser=False, no_local_server=True)
+        f2 = Foundry(authorizers=auths, no_browser=False, no_local_server=True)
         assert isinstance(f2.dlhub_client, DLHubClient)
         assert isinstance(f2.forge_client, Forge)
         assert isinstance(f2.connect_client, MDFConnectClient)
 
-        f3 = Foundry(test_dataset, download=False, authorizers=auths, no_browser=True, no_local_server=False)
+        f3 = Foundry(authorizers=auths, no_browser=True, no_local_server=False)
         assert isinstance(f3.dlhub_client, DLHubClient)
         assert isinstance(f3.forge_client, Forge)
         assert isinstance(f3.connect_client, MDFConnectClient)
 
 
 def test_list():
-    f = Foundry(test_dataset, download=False, authorizers=auths)
+    f = Foundry(authorizers=auths)
     ds = f.list()
     assert isinstance(ds, pd.DataFrame)
     assert len(ds) > 0
 
 
 def test_search():
-    f = Foundry(test_dataset, download=False, authorizers=auths)
+    f = Foundry(authorizers=auths)
     q = "Elwood"
     ds = f.search(q)
 
@@ -245,20 +245,25 @@ def test_search():
 
 
 def test_metadata_pull():
-    f = Foundry(test_dataset, download=False, authorizers=auths)
+    f = Foundry(authorizers=auths)
+    assert f.dc == {}
+    f = f.load(test_dataset, download=False, authorizers=auths)
     assert f.dc["titles"][0]["title"] == expected_title
 
 
 def test_download_https():
-    f = Foundry(test_dataset, download=True, globus=False, authorizers=auths)
+    f = Foundry(authorizers=auths)
     _delete_test_data(f)
+    f = f.load(test_dataset, download=True, globus=False, authorizers=auths)
 
     assert f.dc["titles"][0]["title"] == expected_title
     _delete_test_data(f)
 
 
 def test_dataframe_load():
-    f = Foundry(test_dataset, download=True, globus=False, authorizers=auths)
+    f = Foundry(authorizers=auths)
+    _delete_test_data(f)
+    f = f.load(test_dataset, download=True, globus=False, authorizers=auths)
 
     res = f.load_data()
     X, y = res['train']
@@ -271,7 +276,9 @@ def test_dataframe_load():
 
 
 def test_dataframe_load_split():
-    f = Foundry(test_dataset, download=True, globus=False, authorizers=auths)
+    f = Foundry(authorizers=auths)
+    _delete_test_data(f)
+    f = f.load(test_dataset, download=True, globus=False, authorizers=auths)
 
     res = f.load_data(splits=['train'])
     X, y = res['train']
@@ -284,7 +291,9 @@ def test_dataframe_load_split():
 
 
 def test_dataframe_load_split_wrong_split_name():
-    f = Foundry(test_dataset, download=True, globus=False, authorizers=auths)
+    f = Foundry(authorizers=auths)
+    _delete_test_data(f)
+    f = f.load(test_dataset, download=True, globus=False, authorizers=auths)
 
     with pytest.raises(Exception) as exc_info:
         f.load_data(splits=['chewbacca'])
@@ -297,15 +306,19 @@ def test_dataframe_load_split_wrong_split_name():
 
 @pytest.mark.skip(reason='No clear examples of datasets without splits - likely to be protected against soon.')
 def test_dataframe_load_split_but_no_splits():
-    f = Foundry(test_dataset, download=True, globus=False, authorizers=auths)
+    f = Foundry(authorizers=auths)
+    _delete_test_data(f)
+    f = f.load(test_dataset, download=True, globus=False, authorizers=auths)
 
     with pytest.raises(ValueError):
         f.load_data(splits=['train'])
     _delete_test_data(f)
 
 
 def test_dataframe_load_doi():
-    f = Foundry(test_doi, download=True, globus=False, authorizers=auths)
+    f = Foundry(authorizers=auths)
+    _delete_test_data(f)
+    f = f.load(test_doi, download=True, globus=False, authorizers=auths)
 
     res = f.load_data()
     X, y = res['train']
@@ -319,16 +332,21 @@ def test_dataframe_load_doi():
 
 @pytest.mark.skipif(bool(is_gha), reason="Test does not succeed on GHA - no Globus endpoint")
 def test_download_globus():
-    f = Foundry(test_dataset, download=True, authorizers=auths, no_browser=True, no_local_server=True)
+    f = Foundry(authorizers=auths, no_browser=True, no_local_server=True)
     _delete_test_data(f)
+    f = f.load(test_dataset, download=True)
 
     assert f.dc["titles"][0]["title"] == expected_title
     _delete_test_data(f)
 
 
 @pytest.mark.skipif(bool(is_gha), reason="Test does not succeed on GHA - no Globus endpoint")
 def test_globus_dataframe_load():
-    f = Foundry(test_dataset, download=True, authorizers=auths, no_browser=True, no_local_server=True)
+    f = Foundry(authorizers=auths, no_browser=True, no_local_server=True)
+
+    _delete_test_data(f)
+
+    f = f.load(test_dataset, download=True)
 
     res = f.load_data()
     X, y = res['train']
@@ -482,7 +500,9 @@ def test_check_status():
 
 
 def test_to_pytorch():
-    f = Foundry(test_dataset, download=True, globus=False, authorizers=auths, no_browser=True, no_local_server=True)
+    f = Foundry(authorizers=auths, no_browser=True, no_local_server=True)
+    _delete_test_data(f)
+    f = f.load(test_dataset, download=True, globus=False, authorizers=auths)
 
     raw = f.load_data()
 
@@ -495,7 +515,9 @@ def test_to_pytorch():
 
 
 def test_to_tensorflow():
-    f = Foundry(test_dataset, download=True, globus=False, authorizers=auths, no_browser=True, no_local_server=True)
+    f = Foundry(authorizers=auths, no_browser=True, no_local_server=True)
+    _delete_test_data(f)
+    f = f.load(test_dataset, download=True, globus=False, authorizers=auths)
 
     raw = f.load_data()