Skip to content

GH-43532: [Python] Remove usage of deprecated pkg_resources in setup.py#43602

Merged
pitrou merged 1 commit intoapache:mainfrom
tlm365:remove-pkg-resources
Aug 8, 2024
Merged

GH-43532: [Python] Remove usage of deprecated pkg_resources in setup.py#43602
pitrou merged 1 commit intoapache:mainfrom
tlm365:remove-pkg-resources

Conversation

@tlm365
Copy link
Copy Markdown
Contributor

@tlm365 tlm365 commented Aug 7, 2024

Rationale for this change

Closes #43532.

What changes are included in this PR?

Remove deprecated pkg_resources.resource_filename('numpy', 'core/include'), replace with numpy.get_include()

Are these changes tested?

Test by CI.

Are there any user-facing changes?

No

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 7, 2024

⚠️ GitHub issue #43532 has been automatically assigned in GitHub to PR creator.

@tlm365 tlm365 marked this pull request as draft August 7, 2024 08:58
@tlm365 tlm365 force-pushed the remove-pkg-resources branch 3 times, most recently from d1bce93 to b700766 Compare August 7, 2024 09:41
@tlm365
Copy link
Copy Markdown
Contributor Author

tlm365 commented Aug 7, 2024

@jorisvandenbossche Could you take a look please,
I choose use importlib.resources() (1) instead of numpy.get_include() (2) because:
(2) need import numpy, then some current release test will fail (eg: ci/scripts/release_test.sh), we need to add numpy as a dependency for requirements-test
(1) failed Python 3.8 CI pipeline (importlib.resources.files available from >=3.9). But, this Python version planning was removed since Arrow v18.0.0, so I think (1) makes more sense. How do you think about it?

@jorisvandenbossche
Copy link
Copy Markdown
Member

@tlm365 thanks for the PR!

I choose use importlib.resources() (1) instead of numpy.get_include() (2) because:
(2) need import numpy, then some current release test will fail (eg: ci/scripts/release_test.sh), we need to add numpy as a dependency for requirements-test

While using numpy.get_include() might indeed require some updates elsewhere, I think we should still try to take that route (it's the official public way provided by numpy to get that information, without needing to rely on internal details, e.g. that it is in core/include).

It might also be that you could move the import numpy inline to where it would be used, and then certain invocations of setup.py might still work without numpy installed.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 7, 2024

⚠️ GitHub issue #43532 has been automatically assigned in GitHub to PR creator.

@tlm365 tlm365 force-pushed the remove-pkg-resources branch from b700766 to ba3d33a Compare August 7, 2024 15:40
@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Aug 7, 2024
@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Aug 7, 2024
@tlm365 tlm365 force-pushed the remove-pkg-resources branch from ba3d33a to e72dec2 Compare August 7, 2024 16:27
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Aug 7, 2024
@tlm365 tlm365 force-pushed the remove-pkg-resources branch from e72dec2 to 61231b5 Compare August 7, 2024 16:40
@tlm365 tlm365 marked this pull request as ready for review August 7, 2024 23:25
@pitrou
Copy link
Copy Markdown
Member

pitrou commented Aug 8, 2024

@github-actions crossbow submit -g python -g wheel

@github-actions

This comment was marked as outdated.

Copy link
Copy Markdown
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

LGTM, I'll just wait for CI results

Signed-off-by: Tai Le Manh <manhtai.lmt@gmail.com>
@pitrou pitrou force-pushed the remove-pkg-resources branch from 61231b5 to 279d16d Compare August 8, 2024 08:39
@pitrou
Copy link
Copy Markdown
Member

pitrou commented Aug 8, 2024

I've rebased on git main, to run CI again.

@pitrou
Copy link
Copy Markdown
Member

pitrou commented Aug 8, 2024

@github-actions crossbow submit -g python -g wheel

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 8, 2024

Revision: 279d16d

Submitted crossbow builds: ursacomputing/crossbow @ actions-5c561e1e95

Task Status
example-python-minimal-build-fedora-conda GitHub Actions
example-python-minimal-build-ubuntu-venv GitHub Actions
test-conda-python-3.10 GitHub Actions
test-conda-python-3.10-cython2 GitHub Actions
test-conda-python-3.10-hdfs-2.9.2 GitHub Actions
test-conda-python-3.10-hdfs-3.2.1 GitHub Actions
test-conda-python-3.10-pandas-latest-numpy-1.26 GitHub Actions
test-conda-python-3.10-pandas-latest-numpy-latest GitHub Actions
test-conda-python-3.10-pandas-nightly-numpy-nightly GitHub Actions
test-conda-python-3.10-substrait GitHub Actions
test-conda-python-3.11 GitHub Actions
test-conda-python-3.11-dask-latest GitHub Actions
test-conda-python-3.11-dask-upstream_devel GitHub Actions
test-conda-python-3.11-hypothesis GitHub Actions
test-conda-python-3.11-pandas-upstream_devel-numpy-nightly GitHub Actions
test-conda-python-3.11-spark-master GitHub Actions
test-conda-python-3.12 GitHub Actions
test-conda-python-3.12-cpython-debug GitHub Actions
test-conda-python-3.8 GitHub Actions
test-conda-python-3.8-pandas-1.0-numpy-1.19 GitHub Actions
test-conda-python-3.9 GitHub Actions
test-conda-python-3.9-pandas-latest-numpy-latest GitHub Actions
test-conda-python-emscripten GitHub Actions
test-cuda-python GitHub Actions
test-debian-12-python-3-amd64 GitHub Actions
test-debian-12-python-3-i386 GitHub Actions
test-fedora-39-python-3 GitHub Actions
test-ubuntu-20.04-python-3 GitHub Actions
test-ubuntu-22.04-python-3 GitHub Actions
wheel-macos-big-sur-cp310-arm64 GitHub Actions
wheel-macos-big-sur-cp311-arm64 GitHub Actions
wheel-macos-big-sur-cp312-arm64 GitHub Actions
wheel-macos-big-sur-cp38-arm64 GitHub Actions
wheel-macos-big-sur-cp39-arm64 GitHub Actions
wheel-macos-catalina-cp310-amd64 GitHub Actions
wheel-macos-catalina-cp311-amd64 GitHub Actions
wheel-macos-catalina-cp312-amd64 GitHub Actions
wheel-macos-catalina-cp38-amd64 GitHub Actions
wheel-macos-catalina-cp39-amd64 GitHub Actions
wheel-manylinux-2-28-cp310-amd64 GitHub Actions
wheel-manylinux-2-28-cp310-arm64 GitHub Actions
wheel-manylinux-2-28-cp311-amd64 GitHub Actions
wheel-manylinux-2-28-cp311-arm64 GitHub Actions
wheel-manylinux-2-28-cp312-amd64 GitHub Actions
wheel-manylinux-2-28-cp312-arm64 GitHub Actions
wheel-manylinux-2-28-cp38-amd64 GitHub Actions
wheel-manylinux-2-28-cp38-arm64 GitHub Actions
wheel-manylinux-2-28-cp39-amd64 GitHub Actions
wheel-manylinux-2-28-cp39-arm64 GitHub Actions
wheel-manylinux-2014-cp310-amd64 GitHub Actions
wheel-manylinux-2014-cp310-arm64 GitHub Actions
wheel-manylinux-2014-cp311-amd64 GitHub Actions
wheel-manylinux-2014-cp311-arm64 GitHub Actions
wheel-manylinux-2014-cp312-amd64 GitHub Actions
wheel-manylinux-2014-cp312-arm64 GitHub Actions
wheel-manylinux-2014-cp38-amd64 GitHub Actions
wheel-manylinux-2014-cp38-arm64 GitHub Actions
wheel-manylinux-2014-cp39-amd64 GitHub Actions
wheel-manylinux-2014-cp39-arm64 GitHub Actions
wheel-windows-cp310-amd64 GitHub Actions
wheel-windows-cp311-amd64 GitHub Actions
wheel-windows-cp312-amd64 GitHub Actions
wheel-windows-cp38-amd64 GitHub Actions
wheel-windows-cp39-amd64 GitHub Actions

@pitrou
Copy link
Copy Markdown
Member

pitrou commented Aug 8, 2024

CI failures are unrelated, I'll merge. Thanks a lot @tlm365 !

@pitrou pitrou merged commit 3420c0d into apache:main Aug 8, 2024
@pitrou pitrou removed the awaiting change review Awaiting change review label Aug 8, 2024
@conbench-apache-arrow
Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 3420c0d.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 26 possible false positives for unstable benchmarks that are known to sometimes produce them.

@tlm365 tlm365 deleted the remove-pkg-resources branch August 9, 2024 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Python] Remove usage of deprecated pkg_resources in setup.py

3 participants