Add python client unit test to Github CI (#663)

Summary:

The python client now runs unittests on Github and lints. 

New mypy and format check now fails so these are fixed here too.

Differential Revision: D70407932

.github/workflows/build-lint-test.yml

@@ -14,52 +14,82 @@ jobs:
 
     steps:
       - uses: actions/checkout@v2
+
       - name: Set up Python
         uses: actions/setup-python@v2
         with:
           python-version: "3.10"
+
       - name: Install dependencies
         run: |
+          python -m pip install --upgrade pip
           pip install ".[dev]"
-      - name: Lint with flake8
-        id: flake8
+          cd clients/python
+          pip install .
+      
+      - name: Lint aepsych with flake8
         run: |
           # stop the build if there are Python syntax errors or undefined names
           flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
           # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
           flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
-      - name: Type check with mypy
-        id: mypy
+      
+      - name: Type check aepsych with mypy
+        if: always()
+        run: mypy --config-file mypy.ini
+
+      - name: Lint sepsych_client with flake8  
         if: always()
         run: |
-          mypy --config-file mypy.ini
+          # stop the build if there are Python syntax errors or undefined names
+          flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
+          # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
+          flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
+        working-directory: clients/python
+
+      - name: Type check aepsych_client with mypy
+        if: always()
+        run: mypy --config-file mypy.ini
+        working-directory: clients/python
+
       - uses: omnilib/ufmt@action-v1
         if: always()
         with:
-          path: aepsych tests tests_gpu
+          path: aepsych tests tests_gpu clients/python
           requirements: requirements-fmt.txt
           python-version: "3.10"
 
-  build-test:
 
+  build-test:
     strategy:
       fail-fast: false
       matrix:
         python-version: ["3.10", "3.11", "3.12"]
         os: [macos-latest, windows-latest, macos-13]
+
     runs-on: ${{ matrix.os }}
 
     steps:
     - uses: actions/checkout@v2
+
     - name: Set up Python ${{ matrix.python-version }}
       uses: actions/setup-python@v2
       with:
         python-version: ${{ matrix.python-version }}
+
     - name: Install dependencies
       run: |
         python -m pip install --upgrade pip
         pip install ".[dev]"
-    - name: Test with unittest
-      run: |
-        cd tests
-        python -m unittest
+        cd  clients/python
+        pip install .
+    
+    - name: Test aepsych with unittest
+      run: python -m unittest
+      working-directory: tests
+
+    - name: Test aepsych python client with unittest
+      if: always()
+      run: python -m unittest
+      working-directory: clients/python/tests
+

clients/python/aepsych_client/client.py

@@ -23,7 +23,7 @@ def __init__(
         ip: Optional[str] = None,
         port: Optional[int] = None,
         connect: bool = True,
-        server: "AEPsychServer" = None,
+        server: Optional["AEPsychServer"] = None,
     ) -> None:
         """Python client for AEPsych using built-in python sockets. By default it connects
         to a localhost server matching AEPsych defaults.
@@ -35,8 +35,8 @@ def __init__(
             server (AEPsychServer, optional): An in-memory AEPsychServer object to connect to.
                 If this is not None, the other arguments will be ignored.
         """
-        self.configs = []
-        self.config_names = {}
+        self.configs: List[int] = []
+        self.config_names: Dict[str, int] = {}
         self.server = server
 
         if server is not None and (ip is not None or port is not None):
@@ -49,12 +49,17 @@ def __init__(
             ip = ip or "0.0.0.0"
             port = port or 5555
 
-            self.socket = socket.socket()
+            self.socket: Optional[socket.socket] = socket.socket()
             if connect:
                 self.connect(ip, port)
+        else:
+            self.socket = None
 
     def load_config_index(self) -> None:
         """Loads the config index when server is not None"""
+        if self.server is None:
+            raise AttributeError("there is no in-memory server")
+
         self.configs = []
         for i in range(self.server.n_strats):
             self.configs.append(i)
@@ -66,6 +71,9 @@ def connect(self, ip: str, port: int) -> None:
             ip (str): IP to connect to.
             port (str): Port to connect on.
         """
+        if self.socket is None:
+            raise AttributeError("client does not have a socket to connect with")
+
         addr = (ip, port)
         self.socket.connect(addr)
 
@@ -76,7 +84,10 @@ def finalize(self) -> None:
 
     def _send_recv(self, message) -> str:
         if self.server is not None:
-            return self.server.handle_request(message)
+            return json.dumps(self.server.handle_request(message))
+
+        if self.socket is None:
+            raise AttributeError("client does not have a socket to connect with")
 
         message = bytes(json.dumps(message), encoding="utf-8")
         self.socket.send(message)
@@ -106,9 +117,8 @@ def ask(
         """
         request = {"message": {"num_points": num_points}, "type": "ask"}
         response = self._send_recv(request)
-        if isinstance(response, str):
-            response = json.loads(response)
-        return response
+
+        return json.loads(response)
 
     def tell(
         self,
@@ -143,7 +153,10 @@ def tell(
         self._send_recv(request)
 
     def configure(
-        self, config_path: str = None, config_str: str = None, config_name: str = None
+        self,
+        config_path: Optional[str] = None,
+        config_str: Optional[str] = None,
+        config_name: Optional[str] = None,
     ) -> None:
         """Configure the server and prepare for data collection.
         Note that either config_path or config_str must be passed.
@@ -174,7 +187,9 @@ def configure(
         if config_name is not None:
             self.config_names[config_name] = idx
 
-    def resume(self, config_id: int = None, config_name: str = None):
+    def resume(
+        self, config_id: Optional[int] = None, config_name: Optional[str] = None
+    ):
         """Resume a previous config from this session. To access available configs,
         use client.configs or client.config_names
 

clients/python/mypy.ini

@@ -0,0 +1,5 @@
+[mypy]
+files = aepsych_client
+show_error_codes = True
+pretty = True
+ignore_missing_imports = True

clients/python/pyproject.toml

@@ -0,0 +1,13 @@
+[build-system]
+requires = ["setuptools>=42", "wheel"]
+build-backend = "setuptools.build_meta"
+
+[tool.usort]
+first_party_detection = false
+
+[tool.black]
+target-version = ["py310"]
+
+[tool.ufmt]
+formatter = "ruff-api"
+sorter = "usort"