Repository: brainglobe/brainreg (language: python)

Issue:
Remove niftyreg binaries from package
In the future we should remove the niftyreg binaries from the packages, and give users clear instructures on how to install niftyreg from `conda-forge`.


Discussion / hints:
Or even better, just move to `conda install -c conda-forge brainreg` as the default installation option.
Perhaps linked to this, but on a windows installation using Mamba, the `niftyreg` binaries are installed in 
`CONDA_PREFIX\Library\bin`

Whereas in your `conda_install_path` method in `niftyreg_binaries.py` references them as being in 
`CONDA_PREFIX\bin`
https://github.com/brainglobe/brainreg/blob/fd6475f90e2cd1f45d961df4f897aaab2905be43/src/brainreg/backend/niftyreg/niftyreg_binaries.py#L17

Moreover, your test `if (bin_path / "reg_aladin").exists()` returns `False` for me, as the only way to get it to return `True` is to add `.exe`
```
if (bin_path / "reg_aladin.exe").exists():
```

So I guess you'd need to separate between Windows and Mac/Linux?  

Perhaps this is not the way, but it's the only way I managed to get Brainreg to work


@willGraham01 is this something you could take a look at?
Will take a look this afternoon & next week. Would like to merge #132 in first before making any changes though.

Fix patch:
diff --git a/src/brainreg/backend/niftyreg/niftyreg_binaries.py b/src/brainreg/backend/niftyreg/niftyreg_binaries.py
index 25fb705..0f9b4a0 100644
--- a/src/brainreg/backend/niftyreg/niftyreg_binaries.py
+++ b/src/brainreg/backend/niftyreg/niftyreg_binaries.py
@@ -6,30 +6,52 @@
 __os_folder_names = {"Linux": "linux_x64", "Darwin": "osX", "Windows": "win64"}
 
 try:
-    os_folder_name = __os_folder_names[platform.system()]
+    os_system_name = platform.system()
+    os_folder_name = __os_folder_names[os_system_name]
 except KeyError:
     raise ValueError(
         f"Platform {platform.system()} is not recognised as a valid platform. "
         f"Valid platforms are : {__os_folder_names.keys()}"
     )
 
+_IS_WINDOWS_OS = os_system_name == "Windows"
 
-def conda_install_path() -> Optional[Path]:
+packaged_binaries_folder = (
+    Path(__file__).parent.parent.parent / "bin" / "nifty_reg"
+)
+
+
+def conda_niftyreg_path() -> Optional[Path]:
     """
     If a conda install of niftyreg is available, return the directory
     containing the niftyreg binaries.
     """
-    if "CONDA_PREFIX" not in os.environ:
-        return None
+    if "CONDA_PREFIX" in os.environ:
+        conda_prefix = Path(os.environ["CONDA_PREFIX"])
 
-    bin_path = Path(os.environ["CONDA_PREFIX"]) / "bin"
-    if (bin_path / "reg_aladin").exists():
-        return bin_path
+        if platform.system() == "Windows":
+            # Install prefix into conda environments on Windows
+            # is CONDA_PREFIX / Library / bin
+            bin_path = conda_prefix / "Library" / "bin"
 
+            # Determine if the binaries are present
+            # Binaries MUST have .exe appended to them in this case
+            if (bin_path / "reg_aladin.exe").exists():
+                return bin_path
+        else:
+            # On MacOS and Linux, binaries are placed into
+            # CONDA_PREFIX / bin
+            bin_path = conda_prefix / "bin"
+
+            # Determine if the binaries are present
+            if (bin_path / "reg_aladin").exists():
+                return bin_path
+    # If the binaries are not installed into the conda environment,
+    # or there is no conda environment, return None as a fail-case
     return None
 
 
-_CONDA_INSTALL_PATH = conda_install_path()
+_CONDA_NIFTYREG_BINARY_PATH = conda_niftyreg_path()
 
 
 def get_binary(program_name: str) -> Path:
@@ -39,10 +61,14 @@ def get_binary(program_name: str) -> Path:
     If niftyreg is installed via conda, use those binaries, otherwise fall
     back on bundled binaries.
     """
-    if _CONDA_INSTALL_PATH is not None:
-        return _CONDA_INSTALL_PATH / program_name
+    if _CONDA_NIFTYREG_BINARY_PATH is not None:
+        bin_path = _CONDA_NIFTYREG_BINARY_PATH / program_name
     else:
-        binaries_folder = (
-            Path(__file__).parent.parent.parent / "bin" / "nifty_reg"
-        )
-        return Path(binaries_folder) / os_folder_name / program_name
+        bin_path = packaged_binaries_folder / os_folder_name / program_name
+
+    # Append exe label to Windows executables
+    # It looks like subprocess is actually able to cope without the .exe
+    # appended, but just to be safe we'll include it on Windows OS calls
+    if _IS_WINDOWS_OS:
+        bin_path = bin_path.parent / f"{bin_path.stem}.exe"
+    return bin_path


Test patch:
diff --git a/.github/workflows/test_and_deploy.yml b/.github/workflows/test_and_deploy.yml
index 4f73e12..4fe8031 100644
--- a/.github/workflows/test_and_deploy.yml
+++ b/.github/workflows/test_and_deploy.yml
@@ -48,9 +48,53 @@ jobs:
         with:
           python-version: ${{ matrix.python-version }}
 
+  test-with-conda:
+    needs: [linting, manifest]
+    name: Test conda-install, on ${{ matrix.os }}
+    runs-on: ${{ matrix.os }}
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ubuntu-latest, macos-latest, windows-latest]
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v3
+
+      - name: Setup mamba environment
+        uses: mamba-org/setup-micromamba@v1
+        with:
+          init-shell: bash
+          environment-name: niftyreg-env
+          create-args: >-
+            python=3.11
+            setuptools
+            setuptools_scm
+            wheel
+
+      - name: Install package into environment
+        shell: bash -el {0}
+        run: |
+          pip install .[dev]
+
+      - name: Run backend tests (no conda binaries)
+        shell: bash -el {0}
+        run: |
+          pytest -v tests/tests/test_backend
+
+      - name: Install niftyreg with conda
+        shell: bash -el {0}
+        run: |
+          micromamba install niftyreg
+
+      - name: Run backend tests(conda binaries present)
+        shell: bash -el {0}
+        run: |
+          pytest -v tests/tests/test_backend
+
   build_sdist_wheel:
     name: Build source distribution and wheel
-    needs: [test]
+    needs: [test, test-with-conda]
     if: github.event_name == 'push' && github.ref_type == 'tag'
     runs-on: ubuntu-latest
     steps:
diff --git a/tests/tests/test_backend/__init__.py b/tests/tests/test_backend/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tests/tests/test_backend/test_niftyreg_detection.py b/tests/tests/test_backend/test_niftyreg_detection.py
new file mode 100644
index 0000000..d8b54b1
--- /dev/null
+++ b/tests/tests/test_backend/test_niftyreg_detection.py
@@ -0,0 +1,83 @@
+import os
+from pathlib import Path
+from typing import Tuple
+
+from brainreg.backend.niftyreg.niftyreg_binaries import (
+    _CONDA_NIFTYREG_BINARY_PATH,
+    get_binary,
+    packaged_binaries_folder,
+)
+
+
+def packaged_binaries_are_used() -> Tuple[bool, Path]:
+    """
+    Return a (bool, Path) tuple which has the values:
+        boolean:
+            TRUE if the get_binary function points to one of the niftyreg
+            binaries that are included with the package.
+            FALSE otherwise.
+        Path:
+            The path to the folder containing the niftyreg binaries that
+            were found.
+    """
+    using_binary = get_binary("reg_aladin")
+    return (
+        packaged_binaries_folder in using_binary.parents,
+        using_binary.parent,
+    )
+
+
+def test_conda_with_niftyreg():
+    """
+    Check the following:
+    - If we are not in a conda environment, the packaged niftyreg binaries
+    are used by the backend
+    - If we are in a conda environment and niftyreg is not conda-installed,
+    the packaged niftyreg binaries are used by the backend
+    - If we are in a conda environment and niftyreg IS conda-installed,
+    the conda-installed binaries are used by the backend.
+
+    We need the assert_msg's to get round mypy thinking we're passing tuples
+    to assert (since Tuples always evaluate to true, and mypy wants to split
+    long code lines).
+    """
+    if "CONDA_PREFIX" not in os.environ:
+        # We are not in a conda envrionment
+        # _CONDA_NIFTYREG_BINARY_PATH should be none
+        assert_msg = (
+            "Not in a conda environment but "
+            "_CONDA_NIFTYREG_BINARY_PATH is non-None: "
+            f"{_CONDA_NIFTYREG_BINARY_PATH}"
+        )
+        assert _CONDA_NIFTYREG_BINARY_PATH is None, assert_msg
+
+        using_packaged_binaries, bin_folder = packaged_binaries_are_used()
+        assert_msg = (
+            "There is no CONDA_PREFIX, "
+            f"but non-packaged binaries in {bin_folder} "
+            "are being used!"
+        )
+        assert using_packaged_binaries, assert_msg
+    else:
+        # We are in a conda environment.
+        # Either this environment does not have niftyreg installed, or it does.
+        if _CONDA_NIFTYREG_BINARY_PATH is None:
+            # Apparently niftyreg is not installed in this environment
+            # Assert the located binaries are the packaged ones
+            using_packaged_binaries, bin_folder = packaged_binaries_are_used()
+            assert_msg = (
+                "Conda environment without niftyreg installed exists, "
+                f"but non-packaged binaries in {bin_folder} "
+                "are being used."
+            )
+            assert using_packaged_binaries, assert_msg
+        else:
+            # We are in a conda environment that _has_ niftyreg conda-installed
+            # Assert that we are using the CONDA-installed binaries
+            using_packaged_binaries, bin_folder = packaged_binaries_are_used()
+            assert_msg = (
+                "Packaged binaries are being used, "
+                "despite niftyreg appearing to be installed by CONDA "
+                f" at {_CONDA_NIFTYREG_BINARY_PATH}"
+            )
+            assert not using_packaged_binaries, assert_msg