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.

Sign up for GitHub

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

removed scipy from build-only dependencies #3538

Merged
merged 13 commits into from
Jul 18, 2024
Merged

removed scipy from build-only dependencies #3538

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a148e9b
removed scipy from build-only dependencies; updated README accordingly
filip-komarzyniec Jun 14, 2024
68197fb
fixed conditions regarding oldest numpy installation on arm64 Windows…
filip-komarzyniec Jun 15, 2024
1dc75de
README rephrased regarding NumPy installation
filip-komarzyniec Jun 15, 2024
c3a1f4a
Revert "fixed conditions regarding oldest numpy installation on arm64…
filip-komarzyniec Jun 27, 2024
1d4beef
pin scipy, need old sparsetools for wheel testing
mpenkov Jul 18, 2024
843d05a
Merge branch 'develop' into 3536_remove_scipy_from_build_only_deps
mpenkov Jul 18, 2024
ee1e3dc
make scipy pin more strict
mpenkov Jul 18, 2024
9256f85
Merge remote-tracking branch 'refs/remotes/filip-komarzyniec/3536_rem…
mpenkov Jul 18, 2024
8df0205
use macos-latest instead of deprecated macos-11
mpenkov Jul 18, 2024
420a415
pin scipy in setup.py
mpenkov Jul 18, 2024
5db44ee
remove redundant scipy pin from build-wheels.yml
mpenkov Jul 18, 2024
0b01e9e
get rid of remaining macos-11 traces in CI workflow
mpenkov Jul 18, 2024
3b1f09e
use macos-12 instead of macos-latest
mpenkov Jul 18, 2024
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
20 changes: 11 additions & 9 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,12 @@ on Wikipedia.
Installation
------------

This software depends on [NumPy and Scipy], two Python packages for
scientific computing. You must have them installed prior to installing
gensim.

It is also recommended you install a fast BLAS library before installing
NumPy. This is optional, but using an optimized BLAS such as MKL, [ATLAS] or
[OpenBLAS] is known to improve performance by as much as an order of
This software depends on [NumPy], a Python package for
scientific computing. Please bear in mind that building NumPy from source
(e.g. by installing gensim on a platform which lacks NumPy .whl distribution)
is a non-trivial task involving [linking NumPy to a BLAS library].
It is recommended to provide a fast one (such as MKL, [ATLAS] or
[OpenBLAS]) which can improve performance by as much as an order of
magnitude. On OSX, NumPy picks up its vecLib BLAS automatically,
so you don’t need to do anything special.

Expand All @@ -69,7 +68,9 @@ Or, if you have instead downloaded and unzipped the [source tar.gz]
package:

```bash
python setup.py install
tar -xvzf gensim-X.X.X.tar.gz
cd gensim-X.X.X/
pip install .
```

For alternative modes of installation, see the [documentation].
Expand Down Expand Up @@ -172,7 +173,8 @@ BibTeX entry:
[documentation and Jupyter Notebook tutorials]: https://github.com/RaRe-Technologies/gensim/#documentation
[Vector Space Model]: https://en.wikipedia.org/wiki/Vector_space_model
[unsupervised document analysis]: https://en.wikipedia.org/wiki/Latent_semantic_indexing
[NumPy and Scipy]: https://scipy.org/install/
[NumPy]: https://numpy.org/install/
[linking NumPy to a BLAS library]: https://numpy.org/devdocs/building/blas_lapack.html
[ATLAS]: https://math-atlas.sourceforge.net/
[OpenBLAS]: https://xianyi.github.io/OpenBLAS/
[source tar.gz]: https://pypi.org/project/gensim/
Expand Down
8 changes: 4 additions & 4 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ requires = [
# order of magnitude slower, so avoid it for now.
#
"Cython>=0.29.32,<3.0.0",
# oldest supported Numpy for this platform is 1.17 but the oldest supported by Gensim
# is 1.18.5, remove the line when they increase oldest supported Numpy for this platform
"numpy==1.18.5; python_version=='3.8' and platform_machine not in 'arm64|aarch64'",
# oldest supported Numpy for non-'arm64|aarch64' architectures is 1.17.*
# as well as for 'Windows' on arm64. The oldest supported Numpy by Gensim
# is 1.18.5. Remove below line when they increase the oldest supported Numpy for those platforms
"numpy==1.18.5; python_version=='3.8' and platform_machine != 'aarch64' and (platform_machine != 'arm64' or platform_system == 'Windows')",
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the motivation behind changing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at the meta package oldest-supported-numpy I see that numpy<1.18.5 will be downloaded for python 3.8 and Windows on arm64 (apart from all non-arm64 and non-aarch64 platforms).

So I believe this check was incomplete resulting in installing unsupported version of Numpy for python3.8 arm64 Windows users.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@piskvorky Do we want to deal with this now? It hasn't been reported yet, so it may not actually be a problem. It's been a while since I've messed with oldest-supported-numpy, so I'm not 100% sure this fix is needed and safe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand the reasoning.
Should I revert this change then?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, let's revert it. Feel free to make a separate PR with that change so we can review it later, after the current release is out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do as advised, thanks!

"oldest-supported-numpy; python_version>'3.8' or platform_machine in 'arm64|aarch64'",
"scipy",
"setuptools",
"wheel",
]
Loading