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

relax meson/meson-python requirements #60089

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

isuruf
Copy link

@isuruf isuruf commented Oct 23, 2024

This makes bugfixes from later meson/meson-python fixes available to build pandas. For eg: python 3.13t support in meson, needs an up-to-date version of meson.

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

cc @rgommers

This makes bugfixes from later meson/meson-python fixes available
to build pandas. For eg: python 3.13t support in meson, needs an
up-to-date version of meson.
@lithomas1 lithomas1 added Build Library building on various platforms Python 3.13 labels Oct 24, 2024
Copy link
Contributor

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks Isuru! @lithomas1 @WillAyd we were looking at this, and these exact pins had not been updated since the initial PR that added Meson support to Pandas. I don't think there's anything specific to these versions beyond them being the most recent ones when that support got merged.

pyproject.toml Outdated
@@ -2,8 +2,8 @@
# Minimum requirements for the build system to execute.
# See https://github.com/scipy/scipy/pull/12940 for the AIX issue.
requires = [
"meson-python==0.13.1",
"meson==1.2.1",
"meson-python>=0.13.1,<0.14",
Copy link
Contributor

Choose a reason for hiding this comment

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

We're at 0.17.0 already (with 0.17.1 coming later today). Normally in main one should not add upper bounds at all, and at most all them in release branches only. So I'd drop the upper bound here completely. If Pandas devs are attached to it, then it should be <0.18.

The lower bound can be left as is or bumped, since 0.13.1 is pretty old by now.

Copy link
Member

Choose a reason for hiding this comment

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

I'd be OK to remove the upper bound

"meson-python==0.13.1",
"meson==1.2.1",
"meson-python>=0.13.1,<0.14",
"meson>=1.2.1,<2",
Copy link
Contributor

Choose a reason for hiding this comment

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

This one looks right to me. Note that newer Meson versions have more shiny features one may want (e.g., you need 1.4.0 to have dependency('numpy') work with numpy 2.0 and up, so could be bumped a bit as well if desired.

Copy link
Member

Choose a reason for hiding this comment

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

I vaguely remember trying to upgrade this in #57895 and running into issues; we might want to keep the pin here below 1.4.0 since that has some fundamental changes to how NumPy is integrated (or on the flip side, bump the min to 1.4.0, although that would be a larger task)

Copy link
Member

Choose a reason for hiding this comment

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

Feel free to bump and make that change as well :)

@lithomas1
Copy link
Member

Thanks for the PR.

I'm OK with bumping, but I'd prefer leaving the pin in.
(This has "protected" us from bugs/breaking changes in the latest meson in the past.)

CI looks to be just red in general across pandas unfortunately.
(If someone doesn't fix the failing builds over the next couple of days, I'll go ahead and take a look myself.)

@rgommers
Copy link
Contributor

I'm OK with bumping, but I'd prefer leaving the pin in.
(This has "protected" us from bugs/breaking changes in the latest meson in the past.)

Hard pins in general are never the right thing to do in main (unless the dependency is too unstable, which meson/meson-python are not), so if you really want to keep an == then I suggest:

  1. Put a comment above it that this is a pre-emptive pin only, and distro maintainers can feel free to drop the pin
  2. Amend your release procedure to bump the pinned version to the latest released one before each release.

If you don't do that, you will end up creating problems for distros, as well as missing out on lots of bug fixes which tend to be needed for users on niche platforms and niche compilers in particular.

@WillAyd
Copy link
Member

WillAyd commented Oct 25, 2024

Also +1 to remove the pin. I think it was really useful when we swapped over to Meson, but its probably outlived its usefulness. I've used Meson on a few other projects with only setting a minimum bound, and haven't run into issues.

@isuruf isuruf marked this pull request as ready for review October 25, 2024 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Library building on various platforms Python 3.13
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants