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

Drop Python 3.8 support and loosen runtime NumPy pin #107

Closed
wants to merge 2 commits into from

Conversation

cbourjau
Copy link
Contributor

@cbourjau cbourjau commented Mar 7, 2024

NumPy started with version 1.25 to default to a C-API which is compatible with older versions of NumPy - namely 1.19. This is great because it allows us to have a more lenient lower bound on the NumPy version than what we have today. However, NumPy 1.25 is not supported on Python 3.8. Rather than introducing a special case for Python 3.8 I think it is reasonable to simply drop support for Python 3.8 which will simultaneously slim down our huge build matrix.

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • requirements: host: numpy>=1.25 must contain a space between the name and the pin, i.e. numpy >=1.25
  • requirements: run: numpy>=1.19 must contain a space between the name and the pin, i.e. numpy >=1.19

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@hmaarrfk
Copy link
Contributor

hmaarrfk commented Mar 7, 2024

just passing by and though I would cross reference the discussion in: conda-forge/conda-forge-pinning-feedstock#4816

you might have something to share there about your usecase.

@cbourjau cbourjau changed the title Drop py38 lower numpy pin Drop Python 3.8 support and loosen runtime NumPy pin Mar 8, 2024
@cbourjau cbourjau marked this pull request as draft March 8, 2024 10:42
@cbourjau
Copy link
Contributor Author

just passing by and though I would cross reference the discussion in: conda-forge/conda-forge-pinning-feedstock#4816

you might have something to share there about your usecase.

I doubt that the use case is general enough to make it a default across the ecosystem. We are seeing environments on the client side that have upper pins on NumPy. A common package that used to introduce such pins is numba <=0.53. There is no technical reason why we would not be able to provide an up-to-date onnxruntime in such environments. Dropping Python 3.8 in this particular feedstock has the added benefit of significantly shrinking the huge build matrix, but if desired we could introduce a special case for Python 3.8.

@xhochy
Copy link
Member

xhochy commented Mar 12, 2024

@cbourjau Can you please rebase this? It doesn't look like it correctly compiles.

@cbourjau cbourjau marked this pull request as ready for review March 13, 2024 21:09
@cbourjau cbourjau requested a review from hmaarrfk as a code owner March 13, 2024 21:09
@hmaarrfk
Copy link
Contributor

hmaarrfk commented Mar 13, 2024

Ideological review: I don't think we should be spending open source resources going beyond the scope of SPEC0 https://scientific-python.org/specs/spec-0000/ Generally speaking, maintainer time is invaluable, and I want to actively discourage maintaining too much backward compatibility. It is a real strain on developers.

According to SPEC0 we should:

  • Drop Python 3.9
  • Set numpy's minimum version to 1.23

My general feeling is that the time we spend here as an open source community updating workflows to stay compatible with numpy 1.19 and python 3.9 is better spent fixing the underlying issues that make users require numba <=0.53.

Though, I understand that this is not practical at times. Given that conda-forge's pinnings are already lower than SPEC0, I would encourage we wait until the conversation is resolved in conda-forge/conda-forge-pinning-feedstock#4816

Many of your users will be trying to co-install:

  • onnxruntime
  • opencv

and will be hit with the same incompatibility matrix as before.

(/rant over)

A practical review

A note, this feedstock would be the first that I see using the numpy compatibility stuff in conda-forge.

I would suggest in the test section, to depend on the "old numpys" you think you are compatible with:

I've done this kind of "test matrix" before with
https://github.com/conda-forge/av-feedstock/blob/main/recipe/meta.yaml#L48

The test I envision looks something like

test:
  requires:
   # We want to make sure we are compatible with pretty old numpy
    - numpy 1.19   # [py==39]
    - numpy 1.20   # [py==310]
    - numpy 1.21   # [py==311]
  command:
    - run some kind of onnxruntime on numpy

I would make sure of two things:

  1. You are actually increasing the practical compatibility matrix.
  2. The environment variable you are adding is taking effect.

I hope this helps.

(edit: grammar)

@cbourjau
Copy link
Contributor Author

@hmaarrfk Thanks for the thorough feedback and sorry for the radio silence! I ran into some intricate issues with multiple NumPy version bounds being introduced. This endeavor did turn into more work than anticipated which probably give yet more credence to your little rant. I am re-evaluating if it is indeed worth the trouble.

@cbourjau
Copy link
Contributor Author

We are now building with NumPy 2.0 (#120), which lowers the NumPy pin as intended by this PR for python >=3.10. I don't think this PR's complexity is worth the lower pin for Python 3.9.

@cbourjau cbourjau closed this May 26, 2024
@cbourjau cbourjau deleted the drop-py38-lower-numpy-pin branch May 26, 2024 12:31
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