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

MNT: Use noarch python {{ python_min }} variable #161

Merged

Conversation

matthewfeickert
Copy link
Member

@matthewfeickert matthewfeickert commented Nov 8, 2024

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • [N/A] 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.

* Use 'python {{ python_min }}' syntax for the python requirements for noarch
  python recipes.
   - c.f. https://conda-forge.org/docs/maintainer/knowledge_base/#noarch-python
* Add recipe/conda_build_config.yaml to override the global python_min with uproot's
  python_min of 3.8.
* Bump build number.
* MNT: Re-rendered with conda-build 24.9.0, conda-smithy 3.44.2, and conda-forge-pinning 2024.11.07.16.38.57

* Use 'python {{ python_min }}' syntax for the 'host' python requirement for
  noarch python recipes.
   - c.f. https://conda-forge.org/docs/maintainer/knowledge_base/#noarch-python
* Bump build number.
@matthewfeickert
Copy link
Member Author

@conda-forge-admin, please rerender

@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.
I do have some suggestions for making it better though...

For recipe/meta.yaml:

  • noarch: python recipes should almost always follow the syntax in our documentation. For the host section of the recipe, you should almost always use python {{ python_min }} for the python entry. For the run section of the recipe, you should almost always use python >={{ python_min }} for the python entry. For the test.requires section of the recipe, you should almost always use python {{ python_min }} for the python entry. You may need to override the python_min variable in the conda_build_config.yaml/variants.yaml if the package requires a newer Python version than the currently supported minimum version on conda-forge.

@matthewfeickert matthewfeickert marked this pull request as draft November 8, 2024 00:19
@conda-forge-admin
Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/11733592006.

@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.

@matthewfeickert
Copy link
Member Author

@conda-forge-admin, please rerender

@matthewfeickert matthewfeickert changed the title MNT: Use noarch python 'host' requirement syntax MNT: Use noarch python {{ python_min }} variable Nov 8, 2024
Comment on lines 15 to 16
python_min:
- '3.9'
Copy link
Member Author

@matthewfeickert matthewfeickert Nov 8, 2024

Choose a reason for hiding this comment

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

You may need to override the python_min variable in the conda_build_config.yaml/variants.yaml if the package requires a newer Python version than the currently supported minimum version on conda-forge.

Okay, so I now need to figure out how to override python_min as to be '3.8'.

Copy link
Member Author

Choose a reason for hiding this comment

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

@beckermr @chrisburr @jakirkham This isn't very clear from https://docs.conda.io/projects/conda-build/en/latest/resources/variants.html#general-pinning-examples (as there's nothing on this in https://conda-forge.org/docs/). Do we need to create a recipe/conda_build_config.yaml that is just

python_min:
- '3.8'

or do we need to include all possible Python information in it, essentially duplicating all information in .ci_support/linux_64_.yaml

pin_run_as_build:
  python:
    min_pin: x.x
    max_pin: x.x
python:
- 3.12.* *_cpython
python_min:
- '3.8'

?

Copy link
Member Author

Choose a reason for hiding this comment

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

cc @dopplershift given his participation (and shared concerns on supporting older Pythons) in conda-forge/conda-forge.github.io#2210. Ryan, maybe you have insight here as to how this works?

Copy link
Member

Choose a reason for hiding this comment

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

Put the value you want in a conda-build config in the feedstock and then rerender.

Copy link
Member

Choose a reason for hiding this comment

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

Hopefully that works. If not, we can try other things.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks very much @beckermr. That indeed does work. 👍

If I find time this evening I will open and Issue and PR to make things a bit more explicit for people who have only been doing this for a few years like me.

Copy link
Member

Choose a reason for hiding this comment

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

Improvements to the docs would certainly be welcome 🙂

Writing good docs seems to be one of the hardest problems in open source. So we welcome all the help we can get 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

It's hard, and also the conda-forge team has an nearly impossible task of covering a huge landscape of complicated infrastructure (that still amazingly "just works"!).

Given that I'm trying to unbreak some Scikit-HEP related builts that might not happen tonight, but my goal is to open up a PR in the next 24 hours to iterate with you all on. 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, so closer to 96 hours, but: conda-forge/conda-forge.github.io#2370

@matthewfeickert
Copy link
Member Author

@conda-forge-admin, please rerender

@matthewfeickert matthewfeickert marked this pull request as ready for review November 8, 2024 00:59
@matthewfeickert matthewfeickert merged commit 54c5c02 into conda-forge:main Nov 8, 2024
4 checks passed
@matthewfeickert matthewfeickert deleted the mnt/revise-feedstock-reqs branch November 8, 2024 01:00
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.

4 participants