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

Move requirements to pyproject.toml #12178

Merged
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
a6b737f
Run linter & autoformatter
hoechenberger Nov 6, 2023
e7a74e1
Move requirements*.txt dependencies to pyproject.toml
hoechenberger Nov 6, 2023
f8a1a47
Adjust all references to requirements*.txt files
hoechenberger Nov 6, 2023
56ddbeb
Add `[dev]` variant
hoechenberger Nov 6, 2023
80a4faa
Merge branch 'main' into move-requirements-to-pyproject.toml
hoechenberger Nov 6, 2023
ae54537
Don't install build dependencies that should be automatically install…
hoechenberger Nov 6, 2023
934f949
Use "build" to build sdist and wheel
hoechenberger Nov 6, 2023
ce62c00
Add Dan as maintainer
hoechenberger Nov 6, 2023
58128f1
Restore accidentally deleted "version" entry
hoechenberger Nov 6, 2023
8ea6ddb
FIX: Try
larsoner Nov 8, 2023
3988faa
FIX: Build
larsoner Nov 8, 2023
fa7a577
FIX: Where
larsoner Nov 8, 2023
2caea06
FIX: Namespaces
larsoner Nov 8, 2023
db1194c
FIX: Submodules
larsoner Nov 8, 2023
9bf4775
FIX: Why not
larsoner Nov 8, 2023
28e5df1
FIX: space
larsoner Nov 8, 2023
d29fc04
FIX: Which
larsoner Nov 8, 2023
f62f460
Merge remote-tracking branch 'upstream/main' into move-requirements-t…
larsoner Nov 8, 2023
f92a0b2
FIX: toml
larsoner Nov 8, 2023
97b09fe
FIX: Reorg
larsoner Nov 9, 2023
e614cb4
FIX: Doc guide
larsoner Nov 9, 2023
2a92f75
FIX: Name
larsoner Nov 9, 2023
c63d11c
FIX: More
larsoner Nov 9, 2023
0ff37f5
FIX: Try again
larsoner Nov 9, 2023
936cda6
FIX: Unify
larsoner Nov 9, 2023
9158870
FIX: Circle
larsoner Nov 9, 2023
dff4ad2
FIX: Bar
larsoner Nov 9, 2023
2cec7e9
FIX: req
larsoner Nov 9, 2023
24b4e73
FIX: Req
larsoner Nov 9, 2023
c6373d4
FIX: Try again
larsoner Nov 13, 2023
c83d06e
FIX: Spacing
larsoner Nov 13, 2023
fafe851
FIX: Tests
larsoner Nov 13, 2023
a48b283
FIX: Try/except
larsoner Nov 13, 2023
2c50bfe
Merge remote-tracking branch 'upstream/main' into move-requirements-t…
larsoner Nov 13, 2023
8dfe427
MAINT: Bye bye notebook
larsoner Nov 13, 2023
17494d6
Merge remote-tracking branch 'upstream/main' into move-requirements-t…
larsoner Nov 13, 2023
ec47ba5
FIX: Which
larsoner Nov 13, 2023
770e225
FIX: Alphabetize
larsoner Nov 14, 2023
6cb5a06
FIX: Versionadded
larsoner Nov 14, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ jobs:
brew install [email protected]
which python
which pip
pip install --upgrade pip setuptools wheel
pip install --upgrade --only-binary "numpy,scipy,dipy,statsmodels" -ve . -r requirements.txt -r requirements_testing.txt -r requirements_testing_extra.txt PyQt6
pip install --upgrade pip
pip install --upgrade --only-binary "numpy,scipy,dipy,statsmodels" -ve .[full,test_extra]
# 3D too slow on Apple's software renderer, and numba causes us problems
pip uninstall -y vtk pyvista pyvistaqt numba
mkdir -p test-results
Expand Down
12 changes: 1 addition & 11 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ jobs:
- os: ubuntu-latest
python: '3.10'
kind: conda
- os: ubuntu-latest
python: '3.10'
kind: notebook
- os: ubuntu-latest
python: '3.11'
kind: pip-pre
Expand All @@ -87,7 +84,6 @@ jobs:
with:
qt: true
pyvista: false
if: matrix.kind != 'notebook'
# Python (if pip)
- uses: actions/setup-python@v4
with:
Expand All @@ -103,19 +99,13 @@ jobs:
miniforge-variant: Mambaforge
use-mamba: ${{ matrix.kind != 'conda' }}
if: ${{ !startswith(matrix.kind, 'pip') }}
- name: 'Install OSMesa VTK variant'
run: |
# TODO: As of 2023/02/28, notebook tests need a pinned mesalib
mamba install -c conda-forge "vtk>=9.2=*osmesa*" "vtk-base>=9.2=*osmesa*" "mesalib=23.1.4" "numpy=1.24.4" "numba=0.57.1"
mamba list
if: matrix.kind == 'notebook'
- run: ./tools/github_actions_dependencies.sh
# Minimal commands on Linux (macOS stalls)
- run: ./tools/get_minimal_commands.sh
if: ${{ startswith(matrix.os, 'ubuntu') }}
- run: ./tools/github_actions_install.sh
- run: ./tools/github_actions_infos.sh
# Check Qt on non-notebook
# Check Qt
- run: ./tools/check_qt_import.sh $MNE_QT_BACKEND
if: ${{ env.MNE_QT_BACKEND != '' }}
- name: Run tests with no testing data
Expand Down
6 changes: 0 additions & 6 deletions MANIFEST.in
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
include *.rst
include LICENSE.txt
include SECURITY.md
include requirements.txt
include requirements_base.txt
include requirements_hdf5.txt
include requirements_testing.txt
include requirements_testing_extra.txt
include requirements_doc.txt
include mne/__init__.py

recursive-include examples *.py
Expand Down
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ clean-cache:

clean: clean-build clean-pyc clean-so clean-ctags clean-cache

wheel_quiet:
$(PYTHON) setup.py -q sdist bdist_wheel
wheel:
$(PYTHON) -m build

sample_data:
@python -c "import mne; mne.datasets.sample.data_path(verbose=True);"
Expand Down Expand Up @@ -57,7 +57,7 @@ codespell: # running manually
check-manifest:
check-manifest -q --ignore .circleci/config.yml,doc,logo,mne/io/*/tests/data*,mne/io/tests/data,mne/preprocessing/tests/data,.DS_Store,mne/_version.py

check-readme: clean wheel_quiet
check-readme: clean wheel
twine check dist/*

nesting:
Expand Down
13 changes: 6 additions & 7 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ stages:
displayName: 'Get Python'
- bash: |
set -eo pipefail
python -m pip install --progress-bar off --upgrade pip setuptools wheel
python -m pip install --progress-bar off -r requirements_base.txt -r requirements_hdf5.txt -r requirements_testing.txt
python -m pip install --progress-bar off --upgrade pip build
python -m pip install --progress-bar off -ve .[hdf5,test]
python -m pip uninstall -yq pytest-qt # don't want to set up display, etc. for this
pre-commit install --install-hooks
displayName: Install dependencies
Expand Down Expand Up @@ -107,8 +107,8 @@ stages:
displayName: 'Get Python'
- bash: |
set -e
python -m pip install --progress-bar off --upgrade pip setuptools wheel
python -m pip install --progress-bar off "mne-qt-browser[opengl] @ git+https://github.com/mne-tools/mne-qt-browser.git@main" pyvista scikit-learn pytest-error-for-skips python-picard "PyQt6!=6.5.1" qtpy nibabel
python -m pip install --progress-bar off --upgrade pip
python -m pip install --progress-bar off "mne-qt-browser[opengl] @ git+https://github.com/mne-tools/mne-qt-browser.git@main" pyvista scikit-learn pytest-error-for-skips python-picard "PyQt6!=6.5.1" qtpy nibabel sphinx-gallery
python -m pip uninstall -yq mne
python -m pip install --progress-bar off --upgrade -e .[test]
displayName: 'Install dependencies with pip'
Expand Down Expand Up @@ -166,11 +166,10 @@ stages:
displayName: 'Get Python'
- bash: |
set -e
python -m pip install --progress-bar off --upgrade pip setuptools wheel
python -m pip install --progress-bar off --upgrade pip
python -m pip install --progress-bar off --upgrade --pre --only-binary=\"numpy,scipy,matplotlib,vtk\" numpy scipy matplotlib vtk
python -c "import vtk"
python -m pip install --progress-bar off --upgrade -r requirements.txt -r requirements_testing.txt -r requirements_testing_extra.txt
python -m pip install -e .
python -m pip install --progress-bar off --upgrade -ve .[full,test_extra]
displayName: 'Install dependencies with pip'
- bash: |
set -e
Expand Down
14 changes: 10 additions & 4 deletions doc/development/contributing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -304,11 +304,11 @@ be reflected the next time you open a Python interpreter and ``import mne``
Finally, we'll add a few dependencies that are not needed for running
MNE-Python, but are needed for locally running our test suite::

$ pip install -r requirements_testing.txt
$ pip install -e .[test]
Copy link
Member Author

Choose a reason for hiding this comment

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

behavior change: now install full install + full set up test dependencies

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should advertise this variant actually because it will force people to install PyQt6. Conda users -- including those of our installers -- will have PyQt5

Copy link
Member

Choose a reason for hiding this comment

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

@drammock this is also an issue with the monolithic dev -- we can't directly recommend using it because it uses full which in turn installs PyQt6

Copy link
Member

@drammock drammock Nov 8, 2023

Choose a reason for hiding this comment

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

Can you point me at a discussion of why this fractionation exists? My naive question is why can't we harmonize what gets installed by the standalone installer, conda-forge recipe and pip (i.e., can't we always install PyQt5 regardless of install route)

Copy link
Member

Choose a reason for hiding this comment

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

PyQt6 isn't available on on conda forge yet. PySide6 doesn't support interactive updates well. We could use PyQt5 everywhere but PyQt6 should be better as it's newer and has more features etc (so why not use it and promote it where we can).

Copy link
Contributor

Choose a reason for hiding this comment

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

PyQt5 doesn't receive any updates or bug fixes. I've encountered text rendering bugs that will never be fixed, so I decided to support only PySide6 in MNELAB. PyQt6 should also work, but I'd really stay away from Qt5 stuff.

👍 for recommending pip over conda.

Copy link
Member

Choose a reason for hiding this comment

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

I'm still not seeing the compelling argument to require pyqt6 when possible and pyqt5 otherwise

Yeah we don't explicitly use any Qt6 features yet but it's more about general GUI library usability, features, etc. Qt6 came out almost 3 years ago so it's disappointing to recommend that people use Qt5. We have no choice for our installers at the moment, though, since PyQt6 isn't available there yet 😞 If we change full to install PyQt5 then people who use pip/venv (and previously used requirements.txt) will experience a bit of a step back.

Stepping back a bit, would it work to change the contribution guidelines to say ...

We could recommend using a venv but I think that's a harder step for most people in terms of remembering to activate, relationship to conda envs, etc. So it depends a bit on how advanced/experienced our audience is for the dev part.

To keep things simple and easy for us (in terms of maintenance) and the less experienced devs (in terms of what they need to do), we could focus on supporting what's required for development assuming you're using our installers. In 1.6 at least these will fortunately come with a lot of what's needed:

https://github.com/mne-tools/mne-installers/blob/46a2a7b205cfdfbc1b6969f6436ee3229fedb9f7/recipes/mne-python/construct.yaml#L157

If we consider others with different sys configs (like those using pip venvs) as "advanced" devs, maybe it's okay to expect them to use pytest SKIP statements to see what they need when they need it. If this is acceptable, then recommending in our docs that people do pip install -e .[test] or pip install -e .[test,docs] might be sufficient and easier than trying to roll an all-encompassing dev option.

Copy link
Member Author

@hoechenberger hoechenberger Nov 8, 2023

Choose a reason for hiding this comment

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

Do we even still need conda? I mean, for testing and in our install instructions.

I believe @cbrnr only uses pip to install everything these days, right?

I personally use conda every day still, but as I'm moving all my development into dev containers, I started using pipx and pip more, for easier setup.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe @cbrnr only uses pip to install everything these days, right?

Yes, I've been using pip (and venv) exclusively for a long time now, and I haven't had any major problems. Note that I haven't really tested PyVista though, since I don't need it in my daily workflow. If we recommend VS Code, by default venvs are auto-activated, so I don't think this will be a huge issue.

Copy link
Member

Choose a reason for hiding this comment

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

We could recommend using a venv but I think that's a harder step for most people in terms of remembering to activate, relationship to conda envs, etc. So it depends a bit on how advanced/experienced our audience is for the dev part.

what I meant by the "however you like" in "please create an empty venv (however you like)" was that e.g., conda users could do

$ conda create -n mnedev pip
$ conda activate mnedev
$ pip install -e .[dev]

...so no need to require new knowledge of venv if they understand at least one way of creating environments. Or stated differently: "recommend that contributors install via pip in their dev environment even if they use conda to manage environments".

To keep things simple and easy for us (in terms of maintenance) and the less experienced devs (in terms of what they need to do), we could focus on supporting what's required for development assuming you're using our installers.

-0.5 on this; the installers contain a LOT of stuff that is not needed for MNE dev work (tensorflow, plotly, etc).


And for building our documentation::

$ pip install -r requirements_doc.txt
$ pip install -e .[doc]
$ conda install graphviz

.. note::
Expand All @@ -329,9 +329,15 @@ To build documentation, you will also require `optipng`_:
- On Windows, unzip :file:`optipng.exe` from the `optipng for Windows`_ archive
into the :file:`doc/` folder. This step is optional for Windows users.

You can also choose to install some optional linters for reStructuredText::
There are additional optional dependencies needed to run various tests, such as
scikit-learn for decoding tests, or nibabel for MRI tests. If you want to run all the
tests, consider using our MNE installers (which provide these dependencies) or pay
attention to the skips that ``pytest`` reports and install the relevant libraries.
For example, this traceback::

$ conda install -c conda-forge sphinx-autobuild doc8
SKIPPED [2] mne/io/eyelink/tests/test_eyelink.py:14: could not import 'pandas': No module named 'pandas'

indicates that ``pandas`` needs to be installed in order to run the Eyelink tests.


.. _basic-git:
Expand Down
1 change: 0 additions & 1 deletion doc/links.inc
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@

.. installation links

.. _requirements file: https://raw.githubusercontent.com/mne-tools/mne-python/main/requirements.txt
.. _NVIDIA CUDA GPU processing: https://developer.nvidia.com/cuda-zone
.. _NVIDIA proprietary drivers: https://www.geforce.com/drivers

Expand Down
19 changes: 12 additions & 7 deletions mne/viz/_brain/tests/test_brain.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
# License: Simplified BSD

import os
import sys
import platform
from pathlib import Path
from shutil import copyfile

Expand Down Expand Up @@ -662,15 +662,19 @@ def test_single_hemi(hemi, renderer_interactive_pyvistaqt, brain_gc):

@testing.requires_testing_data
@pytest.mark.slowtest
def test_brain_save_movie(tmp_path, renderer, brain_gc):
@pytest.mark.parametrize("interactive_state", (False, True))
def test_brain_save_movie(tmp_path, renderer, brain_gc, interactive_state):
"""Test saving a movie of a Brain instance."""
from imageio_ffmpeg import count_frames_and_secs
imageio_ffmpeg = pytest.importorskip("imageio_ffmpeg")
if os.getenv("MNE_CI_KIND", "") == "conda" and platform.system() == "Linux":
pytest.skip("Test broken for unknown reason on conda linux")
larsoner marked this conversation as resolved.
Show resolved Hide resolved

brain = _create_testing_brain(
hemi="lh", time_viewer=False, cortex=["r", "b"]
) # custom binarized
filename = tmp_path / "brain_test.mov"
for interactive_state in (False, True):

try:
# for coverage, we set interactivity
if interactive_state:
brain._renderer.plotter.enable()
Expand All @@ -688,11 +692,12 @@ def test_brain_save_movie(tmp_path, renderer, brain_gc):
filename, time_dilation=1.0, tmin=tmin, tmax=tmax, interpolation="nearest"
)
assert filename.is_file()
_, nsecs = count_frames_and_secs(filename)
_, nsecs = imageio_ffmpeg.count_frames_and_secs(filename)
assert_allclose(duration, nsecs, atol=0.2)

os.remove(filename)
brain.close()
finally:
brain.close()


_TINY_SIZE = (350, 300)
Expand Down Expand Up @@ -1093,7 +1098,7 @@ def test_brain_traces(renderer_interactive_pyvistaqt, hemi, src, tmp_path, brain
# TODO: don't skip on Windows, see
# https://github.com/mne-tools/mne-python/pull/10935
# for some reason there is a dependency issue with ipympl even using pyvista
@pytest.mark.skipif(sys.platform == "win32", reason="ipympl issue on Windows")
@pytest.mark.skipif(platform.system() == "Windows", reason="ipympl issue on Windows")
@testing.requires_testing_data
def test_brain_scraper(renderer_interactive_pyvistaqt, brain_gc, tmp_path):
"""Test a simple scraping example."""
Expand Down
Loading
Loading