migrate `noarch: python` to CFEP-25's new syntax
Per CFEP-25, noarch: python packages should have the syntax
host:
- python {{ python_min }}
run:
- python >={{ python_min }}
test:
requires:
- python {{ python_min }}
This issue has TODO items for the migration:
- [x] make conda-build robust to variants needed for proper yaml (https://github.com/conda/conda-build/pull/5528)
- [x] update CFEP to use a simpler syntax that is more compatible (https://github.com/conda-forge/cfep/pull/58, https://github.com/conda-forge/conda-smithy/pull/2122, https://github.com/conda-forge/conda-forge.github.io/pull/2360)
- [x] update docs (https://github.com/conda-forge/conda-forge.github.io/pull/2355)
- [x] add
python_minvariable to pinnings (https://github.com/conda-forge/conda-forge-pinning-feedstock/pull/6646) - [x] write migrator (https://github.com/regro/cf-scripts/pull/3093)
- [x] ~adjust admin command to use new syntax~ (The admin command only adds the parts in build, so I think we should leave it as is. The linter will warn folks and they will have to adjust it by hand.)
- [x] add hints to smithy (https://github.com/conda-forge/conda-smithy/pull/2115, https://github.com/conda-forge/conda-smithy/pull/2119, https://github.com/conda-forge/conda-smithy/pull/2123)
- [x] soften language, improve docs, use jinja2/context (https://github.com/conda-forge/conda-smithy/pull/2126, https://github.com/conda-forge/conda-forge.github.io/pull/2363, https://github.com/conda-forge/conda-forge.github.io/pull/2365)
- [x] fix bug with jinja2 set statement and linter (https://github.com/conda-forge/conda-smithy/pull/2132)
- [x] fix bug in conda-build that causes bad renders (https://github.com/conda/conda-build/pull/5535)
- [x] fix bug in conda-smithy for feedstock creation with
python_min(https://github.com/conda-forge/conda-smithy/pull/2135, https://github.com/conda-forge/conda-smithy/pull/2144) - [x] fix bug in conda-build for regression on undefined jinja2 variables (https://github.com/conda/conda-build/pull/5538)
- [x] make announcement (did on zulip)
- [x] fix more v1 bugs (https://github.com/conda-forge/conda-smithy/pull/2184)
- [ ] run migrator to completion
I understand the rationale about testing against the minimum supported python version but I'm afraid to miss issues with new python version.
distutils was removed from python 3.12. PEP 594 removed several modules from the standard library in 3.13. This can easily break some older packages.
I have an example recently where the build of taurus-feedstock failed with python 3.13. Issue was linked to a dependency, pint, not supporting 3.13. I had to patch the repodata. If the package had been tested against oldest supported python, I'd not have noticed.
Best would be to test against oldest and newest python version, but I don't think that's possible right now.
Is the testing on minimum supported versions a recommendation? Will it be up to the maintainer? Or more enforced?
The cfep allows overrides of any of these things. So yes, feel free. There are new defaults, not rules that have to be followed.
Worth nothing that for v1 recipes, you'll be able to test both the lowest and the highest supported python version, because the script: test section can be repeated.
The linter fails to substitute python_min into the
host:
- python {{ python_min }}
run:
- python >={{ python_min }}
test:
requires:
- python {{ python_min }}
For example the linting complains the following in PR
- https://github.com/conda-forge/dask-ms-feedstock/pull/20
with
Configuring conda
+ [[ -f /home/conda/feedstock_root/LICENSE.txt ]]
+ cp /home/conda/feedstock_root/LICENSE.txt /home/conda/recipe_root/recipe-scripts-license.txt
+ [[ 0 == 1 ]]
+ conda-build /home/conda/recipe_root -m /home/conda/feedstock_root/.ci_support/linux_64_.yaml --suppress-variables --clobber-file /home/conda/feedstock_root/.ci_support/clobber_linux_64_.yaml --extra-meta flow_run_id=azure_20241107.6.1 remote_url=*** sha=f63d15ead9e3a6962930f62dc4b6bd76b427d26a
WARNING: No numpy version specified in conda_build_config.yaml. Falling back to default numpy value of 1.26
Adding in variants from internal_defaults
Adding in variants from /home/conda/feedstock_root/.ci_support/linux_64_.yaml
Traceback (most recent call last):
File "/opt/conda/lib/python3.12/site-packages/conda_build/metadata.py", line 2003, in _get_contents
rendered = template.render(environment=env)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/conda/lib/python3.12/site-packages/jinja2/environment.py", line 1304, in render
self.environment.handle_exception()
File "/opt/conda/lib/python3.12/site-packages/jinja2/environment.py", line 939, in handle_exception
raise rewrite_traceback_stack(source=source)
File "/home/conda/recipe_root/meta.yaml", line 19, in top-level template code
- python {{ python_min }}
jinja2.exceptions.UndefinedError: 'python_min' is undefined
Full log here:
- https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=1074645&view=logs&j=7b6f2c87-f3a7-5133-8d84-7c03a75d9dfc&t=9eb77fd2-8ddd-5444-8fc0-71cb28dcb736&l=617
This is a conda-build error, not a linter error. It appears you need to rerender the feedstock.
Thanks so much @beckermr for all your work on this, it's a huge improvement!!!
I'm running up against a few hard edges though, and I thought it may be useful to share:
- The linter sounds way to authoritative given that we're still working out some of the details. It would be nice if possible to explain that that these are experimental and optional lints for new functionality. (Also, I doubt it's obvious to most people that rerendering is necessary.)
- In case of packages that don't support the global
python_minwe need to override it, but there's no longer an example in the knowledge base for doing this.
I made a few PRs to help with this. Comments welcome!
Sorry I'm a bit late to this discussion. I have a concern about:
Maintainers may override the minimum Python version
python_mininrecipe/conda_build_config.yamlor change the build configuration as needed to match the constraints of their specific package.
I think it's a really nice feature of the feedstocks that routine updates for new releases can all be accomplished purely within recipe/meta.yaml (and then possibly rerendering). Several of the packages I work with follow the scientific Python versioning strategy or similar, and routinely require updates to python_min which are usually strictly above the global pin.
My concern is that in order to automate these updates, e.g. using Grayskull, now meta.yaml will no longer be sufficient since we also have to routinely edit conda_build_config.yaml. I think that as a maintainer this will result in a lot more annoying work.
What would you think of endorsing a strategy of setting python_min as a Jinja2 template variable at the top like this? This would be way easier to maintain in terms of Grayskull or similar.
For grayskull updates, I think it simply injects the minimum by hand anyways, so this is a non-issue?
Also note that the hint literally says "or change the build configuration as needed to match the constraints of their specific package."
For grayskull updates, I think it simply injects the minimum by hand anyways, so this is a non-issue?
What I take away as the biggest innovations from CFEP-25 are:
- Maintaining a source of truth for the minimum Python version
- Testing on that minimum Python version.
Grayskull does inject information from requires-python into the python constraint, but it currently does neither 1 nor 2.
My point is simply that there's potential for confusion if the knowledge base says to update recipe/conda_build_config.yaml but then we use a different implementation in Grayskull. It would be nice if there was a uniformly recommended way to override it. So I'm wondering if there are any advantages to using recipe/conda_build_config.yaml over instead using a Jinja2 variable?
Ah I see what you mean. Does the jinja2 override actually work? We have python_min in the global pinnings too and I am unsure which item takes precedence when jinja2 evaluates the templates.
My general feeling is that carrying important state in jinja2 is not a best practice, but I do see the advantage of not having conda_build_config,yaml files around for noarch: python recipes from the point-of-view of parsimony.
Does the jinja2 override actually work?
Ya. I'm not so experienced with Jinja2, and I'm no big fan of it either, but this example (same that I linked above) demonstrates a successful Jinja2 override, where without the set it picks up the global pin of 3.9 and the test fails because 3.9 is unsupported.
My general feeling is that carrying important state in jinja2 is not a best practice, but I do see the advantage of not having conda_build_config,yaml files around for
noarch: pythonrecipes from the point-of-view of parsimony.
Yes, agreed. I haven't looked into it, but I wonder if rattler offers a better way? Even if so, we probably need to stick to such basic methods for now.
EDIT: To answer my own question about rattler, the answer is affirmative; they introduce a "context variables" mechanism (source):
no full Jinja2 support: no conditional or
{% set ...support, only string interpolation; variables can be set in the toplevel "context" which is valid YAML
ok I am happy to change the docs and linter to suggest context and/or jinja2. We need to ensure that rattler and conda-build have the same precedence order with respect to variants versus context items.
I tested rattler-build overrides and they work fine. I did however find an odd feature where rattler build treats recipe pins like
requirements:
run:
- python 3.10
as exact equality (eg renders to a spec that is ==3.10) as opposed to what conda-build does (which is adding a .* or equivalently =3.10).
I opened an issue here: https://github.com/prefix-dev/rattler-build/issues/1174
Several of the packages I work with follow the scientific Python versioning strategy or similar, and routinely require updates to
python_minwhich are usually strictly above the global pin.
Indeed. I think that having python_min explicitly in the feedstock recipe is quite powerful as it also serves as a way to validate that you remembered to update the Python lower bound in the feedstock when the corresponding Python package updates the requires-python metadata (c.f. PR https://github.com/conda-forge/conda-forge.github.io/pull/2370). So you now have an automated way of avoiding situations like what PR https://github.com/conda-forge/conda-forge-repodata-patches-feedstock/pull/901 patched. That's pretty great!
Thank you @beckermr for also adding PR https://github.com/conda-forge/conda-forge.github.io/pull/2365 as I agree with @maresb that it is very nice if all the metadata for the feedstock can exist just in recipe/meta.yaml. That being said, I'm noticing in https://github.com/conda-forge/pyhf-feedstock/pull/29 that if feedstocks use the Jinja2 set statement for setting the value of {{ python_min }} the linter treats this as having a problem (https://github.com/conda-forge/pyhf-feedstock/pull/29#issuecomment-2469712655) but the linter doesn't treat the python_min map in recipe/conda_build_config.yaml approach as having an issue (https://github.com/conda-forge/pyhf-feedstock/pull/28) (and to be clear in advance I have not had the time to dig into why yet, so I'm only reporting without doing any helpful work).
The linter also only mentions the use of python_min for
If the package requires a newer Python version than the currently supported minimum version on
conda-forge
but it also allows for supporting patch releases of packages where the last minor release still supported a Python version that has been dropped from the supported global pinning. I can understand maybe not wanting to super advertise that old Pythons can still be used for builds in an effort to get people moving towards supporting more modern Pythons overall, but I personally find that to be pretty useful information.
Ah darn. I know what the bug is. Will fix it.
https://github.com/conda-forge/conda-smithy/pull/2132
Sometimes (?) it's necessary to re-render after adding the new python_min syntax - see e.g. https://conda.discourse.group/t/where-is-python-min-defined/898. Maybe it would be nice for the bot's suggestion message to include a prompt to re-render if necessary? My build failed after updating to the suggested syntax, and I was confused for a few minutes!