fix: code review

Author: ymarcon

.github/workflows/deploy.yml

@@ -26,7 +26,7 @@ jobs:
         run: uv sync --all-extras
 
       - name: Run tests
-        run: uv run pytest
+        run: uv run pytest -m "not integration"
 
       - name: Build package
         run: uv build

Makefile

@@ -22,7 +22,7 @@ clean:
 	rm -rf dist
 
 local-install: clean build
-	pip install ./dist/datashield-*.tar.gz
+	pip install ./dist/datashield_opal-*.tar.gz
 
 local-install-force: clean build
-	pip install ./dist/datashield-*.tar.gz --break-system-packages
+	pip install ./dist/datashield_opal-*.tar.gz --break-system-packages

datashield_opal/impl.py

@@ -88,7 +88,8 @@ def assign_table(
             .query("async", asynchronous)
         )
         if variables is not None:
-            builder.query("variables", f'name.any("{",".join(variables)}")')
+            vars = ",".join([f'"{v}"' for v in variables])
+            builder.query("variables", f"name.any({vars})")
         if identifiers is not None:
             builder.query("identifiers", identifiers)
         if id_name is not None:
@@ -191,8 +192,8 @@ def list_packages(self) -> list:
         def format_method(x):
             return f"{x['pkg']}:{x['version']}" if "pkg" in x and "version" in x else None
 
-        aggregate = [format_method(x) for x in aggregate if "pkg" in x]
-        assign = [format_method(x) for x in assign if "pkg" in x]
+        aggregate = [x for x in [format_method(x) for x in aggregate] if x is not None]
+        assign = [x for x in [format_method(x) for x in assign] if x is not None]
 
         # unique values
         pkgs = list(set(aggregate + assign))
@@ -240,7 +241,7 @@ def rm_workspace(self, name: str) -> list:
     def is_async(self) -> dict:
         return {"aggregate": True, "assign_table": True, "assign_resource": True, "assign_expr": True}
 
-    def keep_alive(self) -> bool:
+    def keep_alive(self) -> None:
         with suppress(Exception):
             self.list_symbols()
 
@@ -279,7 +280,7 @@ def _get_session(self):
             if response.code == 201:
                 self.session = response.from_json()
             else:
-                raise OpalDSError(ValueError("DataSHIELD session creation failed: " + response.code))
+                raise OpalDSError(ValueError(f"DataSHIELD session creation failed: {response.code}"))
         return self.session
 
     def _get(self, ws) -> OpalRequest:

pyproject.toml

@@ -41,9 +41,14 @@ Repository = "https://github.com/datashield/datashield-opal-python"
 Documentation = "https://datashield.github.io/datashield-opal-python"
 "Bug Tracker" = "https://github.com/datashield/datashield-opal-python/issues"
 
+[tool.pytest.ini_options]
+markers = [
+    "integration: marks tests as integration tests (deselect with '-m \"not integration\"')",
+]
+
 [build-system]
 requires = ["hatchling"]
 build-backend = "hatchling.build"
 
 [tool.hatch.build.targets.wheel]
-packages = ["datashield"]
+packages = ["datashield_opal"]

tests/test_api_admin.py

@@ -1,4 +1,5 @@
 from datashield import DSSession, DSLoginBuilder
+import pytest
 
 
 class TestClass:
@@ -16,32 +17,38 @@ def setup_class(cls):
     def teardown_class(cls):
         cls.session.close()
 
+    @pytest.mark.integration
     def test_profiles(self):
         rval = self.session.profiles()
         assert "server1" in rval
         assert rval["server1"]["current"] == "default"
         assert len(rval["server1"]["available"]) > 0
 
+    @pytest.mark.integration
     def test_packages(self):
         rval = self.session.packages()
         print(rval)
         assert True
 
+    @pytest.mark.integration
     def test_methods(self):
         rval = self.session.methods()
         print(rval)
         assert True
 
+    @pytest.mark.integration
     def test_tables(self):
         rval = self.session.tables()
         print(rval)
         assert True
 
+    @pytest.mark.integration
     def test_resources(self):
         rval = self.session.resources()
         print(rval)
         assert True
 
+    @pytest.mark.integration
     def test_workspaces(self):
         rval = self.session.workspaces()
         print(rval)

tests/test_api_analysis.py

@@ -1,3 +1,4 @@
+import pytest
 from datashield import DSSession, DSLoginBuilder, DSError
 
 
@@ -13,6 +14,7 @@ def setup_method(self, method):
     def teardown_method(self, method):
         self.session.close()
 
+    @pytest.mark.integration
     def test_assign_tables(self):
         try:
             self.session.assign_table(
@@ -28,6 +30,7 @@ def test_assign_tables(self):
             print(self.session.get_errors())
             raise ValueError("Failed to assign tables") from e
 
+    @pytest.mark.integration
     def test_assign_table(self):
         try:
             self.session.assign_table("df", table="CNSIM.CNSIM1", asynchronous=False)
@@ -41,6 +44,7 @@ def test_assign_table(self):
             print(self.session.get_errors())
             raise ValueError("Failed to assign table") from e
 
+    @pytest.mark.integration
     def test_assign_resources(self):
         try:
             self.session.assign_resource(
@@ -64,6 +68,7 @@ def test_assign_resources(self):
             print(self.session.get_errors())
             raise ValueError("Failed to assign resources") from e
 
+    @pytest.mark.integration
     def test_assign_resource(self):
         try:
             self.session.assign_resource("client", resource="RSRC.CNSIM1", asynchronous=False)

tests/test_impl.py

@@ -21,15 +21,18 @@ def setup_class(cls):
     def teardown_class(cls):
         cls.session.close()
 
+    @pytest.mark.integration
     def test_driver(self):
         conn = self.conn
         assert conn.name == "server1"
 
+    @pytest.mark.integration
     def test_workspaces(self):
         conn = self.conn
         workspaces = conn.list_workspaces()
         assert type(workspaces) is list
 
+    @pytest.mark.integration
     def test_profiles(self):
         conn = self.conn
         profiles = conn.list_profiles()
@@ -39,6 +42,7 @@ def test_profiles(self):
         assert type(profiles["available"]) is list
         assert profiles["current"] == "default"
 
+    @pytest.mark.integration
     def test_methods(self):
         conn = self.conn
         methods = conn.list_methods(type="assign")
@@ -62,6 +66,7 @@ def test_methods(self):
         names = [x["name"] for x in methods]
         assert "meanDS" in names
 
+    @pytest.mark.integration
     def test_packages(self):
         conn = self.conn
         pkgs = conn.list_packages()
@@ -72,20 +77,23 @@ def test_packages(self):
         assert "resourcer" in names
         assert "dsBase" in names
 
+    @pytest.mark.integration
     def test_tables(self):
         conn = self.conn
         tables = conn.list_tables()
         assert type(tables) is list
         assert "CNSIM.CNSIM1" in tables
         assert conn.has_table("CNSIM.CNSIM1")
 
+    @pytest.mark.integration
     def test_resources(self):
         conn = self.conn
         resources = conn.list_resources()
         assert type(resources) is list
         assert "RSRC.CNSIM1" in resources
         assert conn.has_resource("RSRC.CNSIM1")
 
+    @pytest.mark.integration
     def test_assign_expr(self):
         conn = self.conn
         res = conn.assign_expr("x", "c(1, 2, 3)", asynchronous=False)
@@ -110,6 +118,7 @@ def test_assign_expr(self):
         assert type(symbols) is list
         assert len(symbols) == 0
 
+    @pytest.mark.integration
     def test_assign_table(self):
         conn = self.conn
         try:
@@ -138,6 +147,7 @@ def test_assign_table(self):
             print(e.get_error())
             raise ValueError("Assign table test failed") from e
 
+    @pytest.mark.integration
     def test_assign_resource(self):
         conn = self.conn
         try:
@@ -167,6 +177,7 @@ def test_assign_resource(self):
             print(e.get_error())
             raise ValueError("Assign resource test failed") from e
 
+    @pytest.mark.integration
     def test_aggregate(self):
         conn = self.conn
         try:
@@ -196,6 +207,7 @@ def test_aggregate(self):
             print(e.get_error())
             raise ValueError("Aggregate test failed") from e
 
+    @pytest.mark.integration
     def test_aggregate_function_not_allowed(self):
         conn = self.conn
         with pytest.raises(DSError) as exc_info: