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

MAINT: Fix setup #945

Merged
merged 3 commits into from
Jan 18, 2022
Merged

MAINT: Fix setup #945

merged 3 commits into from
Jan 18, 2022

Conversation

larsoner
Copy link
Member

On latest main in a clean env, pip install -ve. does not work:

$ python -m venv create mnebids
$ source mnebids/bin/activate
$ cd mne-bids
$ pip install -ve .
Using pip 20.3.4 from /home/larsoner/python/mnebids/lib/python3.10/site-packages/pip (python 3.10)
Non-user install because user site-packages disabled
...
      File "/home/larsoner/python/mnebids/lib/python3.10/site-packages/setuptools/config.py", line 515, in _parse_version
        version = self._parse_attr(value, self.package_dir)
      File "/home/larsoner/python/mnebids/lib/python3.10/site-packages/setuptools/config.py", line 349, in _parse_attr
        module = import_module(module_name)
      File "/usr/lib/python3.10/importlib/__init__.py", line 126, in import_module
        return _bootstrap._gcd_import(name[level:], package, level)
...
      File "/home/larsoner/python/mne-bids/mne_bids/__init__.py", line 5, in <module>
        from mne_bids.report import make_report
      File "/home/larsoner/python/mne-bids/mne_bids/report.py", line 10, in <module>
        import numpy as np
    ModuleNotFoundError: No module named 'numpy'

This import is attempted because version is not supplied. (This failure was originally seen on MNE-Python CircleCI).One solution is to change this:

SETUP_REQUIRES = ["setuptools >= 46.4.0"]

to

SETUP_REQUIRES = ["setuptools >= 46.4.0", "numpy", "jinja2", "mne"]

But a cleaner solution I think is just to explicitly parse the version from __init__.py. Then the command works correctly, presumably because of pip magic to know what the dependencies of the library are (?), it actually finds the same set (plus what mne-0.24.1 requires):

...
Installing collected packages: numpy, scipy, MarkupSafe, mne, jinja2, mne-bids
...
Successfully installed MarkupSafe-2.0.1 jinja2-3.0.3 mne-0.24.1 mne-bids numpy-1.22.1 scipy-1.7.3
Removed build tracker: '/tmp/pip-req-tracker-s0jv846l'

Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

Thanks for looking into a fix, could you then please remove this line?

version = attr: mne_bids.__version__

Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

Thanks! +1 to merge when CIs are green

@larsoner
Copy link
Member Author

Fixed that and a bug with parse. Using this branch on CircleCI we get a nice install now:

...
Building wheels for collected packages: numpydoc, pydata-sphinx-theme, sphinx-gallery, mne-bids, mne-connectivity, quantities, future
...
Successfully installed ... mne-0.24.1 mne-bids-0.10.dev0 mne-connectivity-0.3.dev0 mne-qt-browser-0.1.7 ...

@codecov
Copy link

codecov bot commented Jan 18, 2022

Codecov Report

Merging #945 (b526237) into main (31a870e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #945   +/-   ##
=======================================
  Coverage   94.39%   94.39%           
=======================================
  Files          25       25           
  Lines        3550     3550           
=======================================
  Hits         3351     3351           
  Misses        199      199           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31a870e...b526237. Read the comment docs.

@larsoner
Copy link
Member Author

Two approvals and green CIs so in it goes. Thanks for the quick reviews @adam2392 @sappelhoff !

@larsoner larsoner merged commit 2bb880a into mne-tools:main Jan 18, 2022
@larsoner larsoner deleted the setup branch January 18, 2022 17:36
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.

3 participants