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

Rebuild for CFEP-25 noarch: python syntax #4

Conversation

regro-cf-autotick-bot
Copy link
Contributor

This PR updates the recipe to use the noarch: python syntax as described in CFEP-25. Please see our documentation for more details.

If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/cf-scripts/actions/runs/12342125756 - please use this URL for debugging.

@conda-forge-admin
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/meta.yaml) and found it was in an excellent condition.

Copy link
Contributor

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

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

Should be ok, once min_python is bumped to 3.10

Copy link
Contributor

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

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

Honestly this just regresses things.. I'm not sure how useful this PR becomes.

As far as I'm aware this is how you do a > min pin.

@@ -27,22 +27,22 @@ outputs:
noarch: python
requirements:
host:
- python >=3.9
- python {{ python_min }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- python {{ python_min }}
- python >=3.10

Choose a reason for hiding this comment

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

Yeah, reading through CFEP-25 I'm not sure I understand the logic of requiring the minimum version on the host? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

See discussion below.

Choose a reason for hiding this comment

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

AFAICT the discussion is on updating the python_min pin, but the bot PR here and elsewhere removes the >= in favour of just requiring the minimum version -- I don't understand this (unless I've misunderstood the syntax).

Copy link
Contributor

Choose a reason for hiding this comment

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

See my comment in the main PR thread: for another PR, this was set to >= so the bot is doing something odd.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops... ^h^h ... the change that I referred to was in run and not in host: there it is also pinned to python {{ python_min }}.

- setuptools >=40.9.0
- wheel
- pip
run:
- python >=3.9
- python >={{ python_min }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- python >={{ python_min }}
- python >=3.10

@orbeckst
Copy link
Contributor

orbeckst commented Dec 16, 2024

The docs https://conda-forge.org/docs/maintainer/knowledge_base/#noarch-python say

All recipes employing noarch: python should usually use the python_min variable per the following example:

and then they recommend (EDIT: not "recommend" but "if you need to override") to set min_python in recipe/meta.yaml or recipe/conda_build_config.yaml

@orbeckst
Copy link
Contributor

orbeckst commented Dec 16, 2024

I think https://github.com/conda-forge/cfep/blob/main/cfep-25.md is what recently changed and they want to use python_min.

Copy link
Contributor

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

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

Figure out how to do to CFEP-25 properly.

@IAlibay
Copy link
Contributor

IAlibay commented Dec 16, 2024

The docs https://conda-forge.org/docs/maintainer/knowledge_base/#noarch-python say

All recipes employing noarch: python should usually use the python_min variable per the following example:

and then they recommend (EDIT: not "recommend" but "if you need to override") to set min_python in recipe/meta.yaml or recipe/conda_build_config.yaml

Ah yes, I saw scipy do this before! we can set it in the meta.yaml, completely went over my head.

@orbeckst
Copy link
Contributor

orbeckst commented Dec 16, 2024

We had some help on conda-forge/staged-recipes#28554 and there we have now

requirements:
  host:
    - python {{ python_min }|
    ...

  run:
    - python >={{ python_min }}

... 

test:
  ... 

  requires:
  - python {{ python_min }}

Copy link
Contributor

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

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

I am undoing my previous change.

@@ -27,22 +27,22 @@ outputs:
noarch: python
requirements:
host:
- python >=3.9
- python {{ python_min }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops... ^h^h ... the change that I referred to was in run and not in host: there it is also pinned to python {{ python_min }}.

The latest release of mdahole2 supports 3.10 as minimal python.
Copy link
Contributor

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

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

Overriding python_min in meta.yml seems to work.

As this is the recommended approach for overriding and because the package requires minimal 3.10, I am approving.

@orbeckst
Copy link
Contributor

@lilyminium @IAlibay are you happy with setting python_min in meta.yml?

Copy link

@lilyminium lilyminium left a comment

Choose a reason for hiding this comment

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

Yep!

@orbeckst orbeckst merged commit 965bb53 into conda-forge:main Dec 17, 2024
4 checks passed
@regro-cf-autotick-bot regro-cf-autotick-bot deleted the noarch_python_min-migration-1_he75a9a branch December 17, 2024 00:46
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.

5 participants