Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix macOS wheel build for datajoint (Issue #249) #406

Merged
merged 30 commits into from
Sep 20, 2024

Conversation

MilagrosMarin
Copy link
Contributor

@MilagrosMarin MilagrosMarin commented Sep 11, 2024

Summary:
This PR addresses issue #249, which involves a wheel build failure for datajoint on macOS 12 when using mamba, and proposes three solutions for macOS users.

Also, this PR:

  • Fix GitHub Actions (GHA) test Build env and run tests on macos-latest
  • Refactor build_env_run_tests.yml workflow.

Context: On macOS, when attempting to install datajoint through a conda/mamba environment using an .yml file (see below), the following error occurs during the installation:

name: aeon_test
channels:
  - conda-forge
  - defaults
dependencies:
  - python>=3.11
  - pip
  - blas>=2.0, <3
  - bottleneck>=1.2.1, <2
  - dash
  - dotmap
  - fastparquet
  - graphviz
  - ipykernel
  - jupyter
  - jupyterlab
  - matplotlib
  - numba>=0.46.0, <1
  - numexpr>=2.6.8, <3
  - numpy>=1.21.0, <2
  - pandas>=1.3
  - plotly
  - pyarrow
  - pydotplus
  - pymysql
  - pyyaml
  - scikit-learn
  - scipy
  - seaborn
  - xarray>=0.12.3, <1
  - pip:
      - datajoint>=0.13.6, <1
      - git+https://github.com/datajoint-company/datajoint-utilities.git
      - opencv-python
error: subprocess-exited-with-error
  × python setup.py bdist_wheel did not run successfully.
  │ exit code: 1
  ╰─> [48 lines of output]
      /Users/milagros/miniconda/envs/aeon_test/lib/python3.12/site-packages/setuptools/__init__.py:88: _DeprecatedInstaller: setuptools.installer and fetch_build_eggs are deprecated.
      !!
              ********************************************************************************
              Requirements should be satisfied by a PEP 517 installer.
              If you are using pip, you can try `pip install --use-pep517`.
              ********************************************************************************
              
...

      OSError: [Errno 66] Directory not empty: '/private/var/folders/h8/_lx50k3d6qx586662kr066h40000gn/T/pip-install-ovdbidfz/datajoint_530cc0190d4342fc91f8abda5f5fdf76/.eggs/watchdog-5.0.2-py3.12-macosx-11.0-arm64.egg/watchdog-5.0.2.dist-info' -> '/private/var/folders/h8/_lx50k3d6qx586662kr066h40000gn/T/pip-install-ovdbidfz/datajoint_530cc0190d4342fc91f8abda5f5fdf76/.eggs/watchdog-5.0.2-py3.12-macosx-11.0-arm64.egg/EGG-INFO'
      [end of output]
  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed cleaning build dir for datajoint
ERROR: ERROR: Failed to build installable wheels for some pyproject.toml based projects (datajoint)
failed
CondaEnvException: Pip failed

setuptools no longer recommends using setup.py for package installation. Specifically, setuptools is now raising warnings that setup.py install is deprecated in favor of using modern standards-based tools such as PEP 517 to build packages.

This deprecation is affecting how datajoint package is resolved during the build process for Apple Silicon (M1/M2) Macs. The python packages listed in the setup.py of datajoint dependency are not properly installed.

Solution: As python evolves to modernize package building and installation, the transition from setup.py to pyproject.toml (following PEP 517 and PEP 518) will help standardize the process. To stay aligned with these best practices, datajoint-python can consider updating its dependency management to use pyproject.toml for smoother builds and installations for mac users.

Temporal solutions are:

  • Build datajoint wheel using pip manually in the shell to bypass the issue:
    pip install datajoint
  • Downgrading setuptools to version 58.2.0 would remove the warning but may create conflicts with other dependencies in the environment [1, 2]
    Passing the --use-pep517 flag to pip to solve the wheel-building issue on macOS [3, 4]

Proposed solution in this PR for macOS installation process:

  • Create a env_macos.yml file for conda/mamba, excluding the pip installation of datajoint and datajoint-utilities.
  • After the conda environment is created, manually install both packages using pip from the shell within the conda/mamba environment, while passing the required flag. This is also indicated in the env_macos.yml for the macOS user reference.

Tests:
Locally using conda, and using GHA with mamba.

Environment details:
macOS: 12, 14.5
python: 3.11, 3.12.6
conda/mamba: installation attempted via conda/mamba env create -f env.yml

Main packages:

setuptools                73.0.1
wheel                     0.44.0
pip                       24.2

@MilagrosMarin MilagrosMarin marked this pull request as ready for review September 11, 2024 20:21
@MilagrosMarin MilagrosMarin self-assigned this Sep 11, 2024
@MilagrosMarin MilagrosMarin changed the title fix wheel build for datajoint on macOS fix wheel build for datajoint failing on macOS Sep 11, 2024
@MilagrosMarin MilagrosMarin changed the title fix wheel build for datajoint failing on macOS fix(GHA): wheel build for datajoint failing on macOS Sep 11, 2024
@MilagrosMarin MilagrosMarin changed the title fix(GHA): wheel build for datajoint failing on macOS [WIP]: fix(GHA): wheel build for datajoint failing on macOS Sep 11, 2024
@MilagrosMarin MilagrosMarin changed the title [WIP]: fix(GHA): wheel build for datajoint failing on macOS Fix macOS wheel build for datajoint (Issue #249) Sep 12, 2024
@glopesdev glopesdev changed the base branch from main to gl-ruff-check September 19, 2024 16:55
@glopesdev glopesdev self-requested a review September 19, 2024 16:59
Copy link
Contributor

@glopesdev glopesdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great as a whole and we can deal with the remaining conversations directly on the CI squash branch, so I'm merging this.

@glopesdev glopesdev merged commit 81bbfa1 into SainsburyWellcomeCentre:gl-ruff-check Sep 20, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants