code quality fixes

bfabric/__init__.py

@@ -1,11 +1,19 @@
 import importlib.metadata
-
-__version__ = importlib.metadata.version("bfabric")
-
 from bfabric.bfabric import Bfabric, BfabricAPIEngineType
 from bfabric.bfabric_config import BfabricAuth, BfabricConfig
 
 
+__all__ = [
+    "Bfabric",
+    "BfabricAPIEngineType",
+    "BfabricAuth",
+    "BfabricConfig",
+]
+
+
+__version__ = importlib.metadata.version("bfabric")
+
+
 endpoints = sorted(
     [
         "annotation",
@@ -41,9 +49,3 @@
 project = 403
 container = project
 application = 217
-
-
-from bfabric.bfabric_legacy import BfabricLegacy
-from bfabric.wrapper_creator.bfabric_wrapper_creator import BfabricWrapperCreator
-from bfabric.wrapper_creator.bfabric_submitter import BfabricSubmitter
-from bfabric.wrapper_creator.bfabric_feeder import BfabricFeeder

bfabric/bfabric.py

@@ -16,10 +16,10 @@
 import base64
 import importlib.metadata
 import logging
-import os
 from contextlib import contextmanager
 from datetime import datetime
 from enum import Enum
+from pathlib import Path
 from pprint import pprint
 from typing import Literal, ContextManager, Any
 
@@ -291,20 +291,17 @@ def get_system_auth(
     have_config_path = config_path is not None
     if not have_config_path:
         # Get default path config file path
-        config_path = os.path.normpath(os.path.expanduser("~/.bfabricpy.yml"))
+        config_path = Path("~/.bfabricpy.yml").expanduser()
 
     # Use the provided config data from arguments instead of the file
-    if not os.path.isfile(config_path):
+    if not config_path.is_file():
         if have_config_path:
             # NOTE: If user explicitly specifies a path to a wrong config file, this has to be an exception
             raise OSError(f"Explicitly specified config file does not exist: {config_path}")
         # TODO: Convert to log
         print(f"Warning: could not find the config file in the default location: {config_path}")
         config = BfabricConfig(base_url=base_url)
-        if login is None and password is None:
-            auth = None
-        else:
-            auth = BfabricAuth(login=login, password=password)
+        auth = None if login is None and password is None else BfabricAuth(login=login, password=password)
 
     # Load config from file, override some of the fields with the provided ones
     else:
@@ -319,9 +316,8 @@ def get_system_auth(
 
     if not config.base_url:
         raise ValueError("base_url missing")
-    if not optional_auth:
-        if not auth or not auth.login or not auth.password:
-            raise ValueError("Authentication not initialized but required")
+    if not optional_auth and (not auth or not auth.login or not auth.password):
+        raise ValueError("Authentication not initialized but required")
 
     if verbose:
         pprint(config)

bfabric/bfabric_legacy.py

@@ -1,9 +1,10 @@
+from __future__ import annotations
 import base64
 import json
 import os
 import sys
 from pprint import pprint
-from typing import Dict, Any
+from typing import Any
 
 import yaml
 from suds.client import Client
@@ -13,13 +14,13 @@
 from bfabric.bfabric_config import BfabricAuth, read_config
 
 
-class BfabricLegacy(object):
+class BfabricLegacy:
     """B-Fabric python3 module
     Implements read and save object methods for B-Fabric wsdl interface
     """
 
-    def warning(self, msg):
-        sys.stderr.write("\033[93m{}\033[0m\n".format(msg))
+    def warning(self, msg) -> None:
+        sys.stderr.write(f"\033[93m{msg}\033[0m\n")
 
     def __init__(
         self,
@@ -31,7 +32,7 @@ def __init__(
         config_env: str = None,
         optional_auth: bool = False,
         verbose: bool = False,
-    ):
+    ) -> None:
         """
         :param login:           Login string for overriding config file
         :param password:        Password for overriding config file
@@ -54,7 +55,7 @@ def __init__(
         config_path = config_path or os.path.normpath(os.path.expanduser("~/.bfabricpy.yml"))
 
         # TODO: Convert to an exception when this branch becomes main
-        config_path_old = config_path or os.path.normpath(os.path.expanduser("~/.bfabricrc.py"))
+        config_path or os.path.normpath(os.path.expanduser("~/.bfabricrc.py"))
         if os.path.isfile(config_path):
             self.warning(
                 "WARNING! The old .bfabricrc.py was found in the home directory. Delete and make sure to use the new .bfabricpy.yml"
@@ -75,7 +76,7 @@ def __init__(
             elif (login is None) and (password is None):
                 self.auth = auth
             else:
-                raise IOError("Must provide both username and password, or neither.")
+                raise OSError("Must provide both username and password, or neither.")
 
         if not self.config.base_url:
             raise ValueError("base server url missing")
@@ -152,7 +153,7 @@ def _get_service(self, endpoint: str) -> Service:
             self.cl[endpoint] = Client(f"{self.config.base_url}/{endpoint}?wsdl", cache=None)
         return self.cl[endpoint].service
 
-    def _perform_request(self, endpoint: str, method: str, plain: bool, params: Dict[str, Any]) -> Any:
+    def _perform_request(self, endpoint: str, method: str, plain: bool, params: dict[str, Any]) -> Any:
         """Performs a request to the given endpoint and returns the result."""
         self.query_counter += 1
         request_params = dict(login=self.auth.login, password=self.auth.password, **params)
@@ -165,7 +166,7 @@ def _perform_request(self, endpoint: str, method: str, plain: bool, params: Dict
         return getattr(response, endpoint)
 
     @staticmethod
-    def print_json(queryres=None):
+    def print_json(queryres=None) -> None:
         """
         This method prints the query result as returned by ``read_object`` in JSON format.
 
@@ -183,7 +184,7 @@ def print_json(queryres=None):
         print(res)
 
     @staticmethod
-    def print_yaml(queryres=None):
+    def print_yaml(queryres=None) -> None:
         """
         This method prints the query result as returned by ``read_object`` in YAML format.
 
@@ -222,7 +223,7 @@ def get_sampleid(self, resourceid=None):
             workunit = self.read_object(endpoint="workunit", obj={"id": resource.workunit._id})[0]
             return self.get_sampleid(resourceid=int(workunit.inputresource[0]._id))
         except:
-            self.warning("fetching sampleid of resource.workunitid = {} failed.".format(resource.workunit._id))
+            self.warning(f"fetching sampleid of resource.workunitid = {resource.workunit._id} failed.")
             return None
 
 

bfabric/engine/engine_zeep.py

@@ -149,6 +149,6 @@ def _zeep_query_append_skipped(query: dict, skipped_keys: list, inplace: bool =
     """
     query_this = copy.deepcopy(query) if not inplace else query
     for key in skipped_keys:
-        if overwrite or (key not in query_this.keys()):
+        if overwrite or (key not in query_this):
             query_this[key] = zeep.xsd.SkipValue
     return query_this

bfabric/errors.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+from typing import Any
+
 
 class BfabricRequestError(Exception):
     """An error that is returned by the server in response to a full request."""
@@ -18,7 +20,7 @@ class BfabricConfigError(RuntimeError):
 
 
 # TODO: Also test for response-level errors
-def get_response_errors(response, endpoint: str) -> list[BfabricRequestError]:
+def get_response_errors(response: Any, endpoint: str) -> list[BfabricRequestError]:
     """
     :param response:  A raw response to a query from an underlying engine
     :param endpoint:  The target endpoint

bfabric/examples/compare_zeep_suds_pagination.py

@@ -19,7 +19,7 @@
 """
 
 
-def report_test_result(rez: bool, prefix: str):
+def report_test_result(rez: bool, prefix: str) -> None:
     if rez:
         print("--", prefix, "test passed --")
     else:

bfabric/examples/compare_zeep_suds_query.py

@@ -68,7 +68,7 @@ def calc_both(auth: BfabricAuth, config: BfabricConfig, endpoint: str, query: di
 ######################
 
 
-def raw_test(auth: BfabricAuth, config: BfabricConfig, endpoint, query):
+def raw_test(auth: BfabricAuth, config: BfabricConfig, endpoint, query) -> None:
     print("Testing raw XML match for", endpoint, query)
     retZeep, retSuds = calc_both(auth, config, endpoint, query, raw=True)
     assert len(retZeep) == len(retSuds)
@@ -102,7 +102,7 @@ def recursive_get_types(generic_container) -> set:
         return {type(generic_container)}
 
 
-def basic_data_type_match_test(auth, config, endpoint, query):
+def basic_data_type_match_test(auth, config, endpoint, query) -> None:
     print("Testing data types for", endpoint, query)
     retZeepDict, retSudsDict = calc_both(auth, config, endpoint, query, raw=False)
     typesZeep = recursive_get_types(retZeepDict)
@@ -160,7 +160,7 @@ def parsed_data_match_test(
     drop_empty: bool = True,
     drop_underscores_suds: bool = True,
     log_file_path: str = None,
-):
+) -> None:
     print("Testing parsed data match for", endpoint, query)
     retZeepDict, retSudsDict = calc_both(auth, config, endpoint, query, raw=False)
 
@@ -172,9 +172,8 @@ def parsed_data_match_test(
         map_element_keys(retSudsDict, {"_id": "id", "_classname": "classname", "_projectid": "projectid"}, inplace=True)
 
     if log_file_path is not None:
-        with open(log_file_path, "w") as f:
-            with redirect_stdout(f):
-                matched = recursive_comparison(retZeepDict, retSudsDict, prefix=[])
+        with open(log_file_path, "w") as f, redirect_stdout(f):
+            matched = recursive_comparison(retZeepDict, retSudsDict, prefix=[])
     else:
         matched = recursive_comparison(retZeepDict, retSudsDict, prefix=[])
 

bfabric/experimental/__init__.py

@@ -1 +1,3 @@
 from .multi_query import MultiQuery
+
+__all__ = ["MultiQuery"]

bfabric/results/pandas_helper.py

@@ -1,5 +1,6 @@
+from __future__ import annotations
 import pandas as pd
-from typing import Any, List, Dict
+from typing import Any
 
 
 def _stringify(a: Any) -> Any:
@@ -10,7 +11,7 @@ def _stringify(a: Any) -> Any:
     Convert variable to a string if it is of non-basic data type, otherwise keep it as it is
     TODO: Make a better separation between what is and what is not a basic data type
     """
-    if isinstance(a, list) or isinstance(a, dict) or isinstance(a, tuple):
+    if isinstance(a, (list, dict, tuple)):
         return str(a)
     else:
         return a
@@ -24,7 +25,7 @@ def _stringify_dict(d: dict) -> dict:
     return {k: _stringify(v) for k, v in d.items()}
 
 
-def list_dict_to_df(l: List[Dict]) -> pd.DataFrame:
+def list_dict_to_df(l: list[dict]) -> pd.DataFrame:
     """
     :param l: A list of dictionaries
     :return:  Pandas dataframe, where every list element is a new row

bfabric/scripts/bfabric_executable_submitter_functionalTest.py

@@ -35,7 +35,7 @@
 from optparse import OptionParser
 
 
-def main():
+def main() -> None:
 
     parser = OptionParser(usage="usage: %prog -j <externaljobid>", version="%prog 1.0")
 

bfabric/scripts/bfabric_executable_submitter_gridengine.py

@@ -36,7 +36,7 @@
 from bfabric import BfabricSubmitter
 
 
-def main():
+def main() -> None:
 
     parser = OptionParser(usage="usage: %prog -j <externaljobid>", version="%prog 1.0")
 

bfabric/scripts/bfabric_executable_submitter_slurm.py

@@ -37,7 +37,7 @@
 from bfabric import BfabricSubmitter
 
 
-def main():
+def main() -> None:
 
     parser = OptionParser(usage="usage: %prog -j <externaljobid>", version="%prog 1.0")
 

bfabric/scripts/bfabric_executable_wrappercreator.py

@@ -22,7 +22,6 @@
 # $HeadURL: http://fgcz-svn/repos/scripts/trunk/linux/bfabric/apps/python/wrapper_creator_yaml.py $
 # $Id: wrapper_creator_yaml.py 2397 2016-09-06 07:04:35Z cpanse $
 
-import os
 import sys
 from bfabric import BfabricWrapperCreator
 

bfabric/scripts/bfabric_feeder_resource_autoQC.py

@@ -26,7 +26,7 @@ class AutoQC:
     feeder for autoQC raw files
     """
 
-    def __init__(self):
+    def __init__(self) -> None:
         self.bfabric_storageid = 2
         self.client = Bfabric.from_config(verbose=True)
         self.bfabric_application_ids = self.client.config.application_ids
@@ -187,11 +187,11 @@ def resource_check(self, projectid: int, name: str, workunitid: int, filename: s
             res = self.client.save(endpoint="resource", obj=query).to_list_dict()
 
             query = {"id": workunitid, "status": "available"}
-            res2 = self.client.save(endpoint="workunit", obj=query).to_list_dict()
+            self.client.save(endpoint="workunit", obj=query).to_list_dict()
 
         return res[0]["id"]
 
-    def feed(self, line):
+    def feed(self, line) -> None:
         """
         feeds one line example:
         :param line:
@@ -250,10 +250,10 @@ class TestCaseAutoQC(unittest.TestCase):
 
     BF = AutoQC()
 
-    def setUp(self):
+    def setUp(self) -> None:
         pass
 
-    def test_feed(self):
+    def test_feed(self) -> None:
         line = "61cf7e172713344bdf6ebe5b1ed61d99;1549963879;306145606;p2928/Proteomics/QEXACTIVEHF_2/ciuffar_20190211_190211_TNF_PRM_rT_again_AQUA_LHration/20190211_013_autoQC4L.raw"
         # self.BF.feed(line)
         line = "efdf5e375d6e0e4e4abf9c2b3e1e97d5;1542134408;59129652;p1000/Proteomics/QEXACTIVEHF_2/tobiasko_20181113/20181113_003_autoQC01.raw"

bfabric/scripts/bfabric_list_not_existing_storage_directories.py

@@ -11,23 +11,26 @@
 http://fgcz-bfabric.uzh.ch/bfabric/executable?wsdl
 """
 from __future__ import annotations
-import os
+
+from pathlib import Path
 
 from bfabric import Bfabric
 
-ROOTDIR = "/srv/www/htdocs/"
+ROOTDIR = Path("/srv/www/htdocs/")
 
 
 def list_not_existing_storage_dirs(client: Bfabric, technologyid: int = 2) -> None:
+    """Lists not existing storage directories for a given technologyid."""
     results = client.read(endpoint="container", obj={"technologyid": technologyid}).to_list_dict()
     container_ids = sorted({x["id"] for x in results})
 
     for cid in container_ids:
-        if not os.path.isdir(os.path.join(ROOTDIR, f"p{cid}")):
+        if not (ROOTDIR / f"p{cid}").is_dir():
             print(cid)
 
 
 def main() -> None:
+    """Parses CLI arguments and calls `list_not_existing_storage_dirs`."""
     client = Bfabric.from_config(verbose=True)
     list_not_existing_storage_dirs(client=client, technologyid=2)
     list_not_existing_storage_dirs(client=client, technologyid=4)

bfabric/scripts/bfabric_save_csv2dataset.py

@@ -37,6 +37,7 @@
 
 
 def polars_to_bfabric_type(dtype: pl.DataType) -> str | None:
+    """Returns the B-Fabric type for a given Polars data type, defaulting to String if no correspondence is found."""
     if str(dtype).startswith("Int"):
         return "Integer"
     elif str(dtype).startswith("String"):
@@ -48,6 +49,7 @@ def polars_to_bfabric_type(dtype: pl.DataType) -> str | None:
 
 
 def polars_to_bfabric_dataset(data: pl.DataFrame) -> dict[str, list[dict[str, int | str | float]]]:
+    """Converts a Polars DataFrame to a B-Fabric dataset representation."""
     attributes = [
         {"name": col, "position": i + 1, "type": polars_to_bfabric_type(data[col].dtype)}
         for i, col in enumerate(data.columns)