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 #137

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/12958409209 - please use this URL for debugging.

@conda-forge-admin
Copy link
Contributor

conda-forge-admin commented Jan 24, 2025

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/meta.yaml) and found some lint.

Here's what I've got...

For recipe/meta.yaml:

For recipe/meta.yaml:

  • ℹ️ noarch: python recipes should usually follow the syntax in our documentation for specifying the Python version.
    • For the test.requires section of <output 1 output, you should usually use python {{ python_min }} for the python entry.
    • For the test.requires section of <output 2 output, you should usually use python {{ python_min }} for the python entry.
    • If the package requires a newer Python version than the currently supported minimum version on conda-forge, you can override the python_min variable by adding a Jinja2 set statement at the top of your recipe (or using an equivalent context variable for v1 recipes).
  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). This parser is not currently used by conda-forge, but may be in the future. We are collecting information to see which recipes are compatible with grayskull.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/13011294007. Examine the logs at this URL for more detail.

@beckermr
Copy link
Member

@conda-forge-admin rerender

@conda-forge-admin
Copy link
Contributor

conda-forge-admin commented Jan 28, 2025

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 usually follow the syntax in our documentation for specifying the Python version.
    • For the test.requires section of <output 1 output, you should usually use python {{ python_min }} for the python entry.
    • For the test.requires section of <output 2 output, you should usually use python {{ python_min }} for the python entry.
    • If the package requires a newer Python version than the currently supported minimum version on conda-forge, you can override the python_min variable by adding a Jinja2 set statement at the top of your recipe (or using an equivalent context variable for v1 recipes).
  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). This parser is not currently used by conda-forge, but may be in the future. We are collecting information to see which recipes are compatible with grayskull.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/13014995894. Examine the logs at this URL for more detail.

@beckermr
Copy link
Member

@conda-forge-admin rerender

@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/13012099842. Examine the logs at this URL for more detail.

@beckermr
Copy link
Member

@conda-forge-admin rerender

@beckermr beckermr added automerge Merge the PR when CI passes and removed automerge Merge the PR when CI passes labels Jan 28, 2025
@beckermr beckermr added the automerge Merge the PR when CI passes label Jan 28, 2025
recipe/meta.yaml Outdated Show resolved Hide resolved
@beckermr beckermr removed the automerge Merge the PR when CI passes label Jan 28, 2025
recipe/meta.yaml Outdated Show resolved Hide resolved
@beckermr
Copy link
Member

@conda-forge-admin rerender

@conda-forge-admin
Copy link
Contributor

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

I tried to rerender for you but ran into some issues. Please check the output logs of the GitHub Actions workflow below for more details. You can also ping conda-forge/core (using the @ notation) for further assistance or you can try rerendering locally.

The following suggestions might help debug any issues:

  • Is the recipe/{meta.yaml,recipe.yaml} file valid?
  • If there is a recipe/conda-build-config.yaml file in the feedstock make sure that it is compatible with the current global pinnnings.
  • Is the fork used for this PR on an organization or user GitHub account? Automated rerendering via the webservices admin bot only works for user GitHub accounts.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/13014922711. Examine the logs at this URL for more detail.

@beckermr
Copy link
Member

@conda-forge-admin rerender

@beckermr
Copy link
Member

@conda-forge-admin rerender

@conda-forge-admin
Copy link
Contributor

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

I tried to rerender for you but ran into some issues. Please check the output logs of the GitHub Actions workflow below for more details. You can also ping conda-forge/core (using the @ notation) for further assistance or you can try rerendering locally.

The following suggestions might help debug any issues:

  • Is the recipe/{meta.yaml,recipe.yaml} file valid?
  • If there is a recipe/conda-build-config.yaml file in the feedstock make sure that it is compatible with the current global pinnnings.
  • Is the fork used for this PR on an organization or user GitHub account? Automated rerendering via the webservices admin bot only works for user GitHub accounts.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/13015001739. Examine the logs at this URL for more detail.

recipe/meta.yaml Outdated Show resolved Hide resolved
@conda-forge-admin
Copy link
Contributor

conda-forge-admin commented Jan 28, 2025

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/meta.yaml) and found some lint.

Here's what I've got...

For recipe/meta.yaml:

  • ❌ The recipe must have a build/number section.

For recipe/meta.yaml:

  • ℹ️ noarch: python recipes should usually follow the syntax in our documentation for specifying the Python version.
    • For the test.requires section of <output 1 output, you should usually use python {{ python_min }} for the python entry.
    • For the test.requires section of <output 2 output, you should usually use python {{ python_min }} for the python entry.
    • If the package requires a newer Python version than the currently supported minimum version on conda-forge, you can override the python_min variable by adding a Jinja2 set statement at the top of your recipe (or using an equivalent context variable for v1 recipes).
  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). This parser is not currently used by conda-forge, but may be in the future. We are collecting information to see which recipes are compatible with grayskull.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/13015278010. Examine the logs at this URL for more detail.

@beckermr
Copy link
Member

@conda-forge-admin rerender

@conda-forge-admin
Copy link
Contributor

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

I tried to rerender for you but ran into some issues. Please check the output logs of the GitHub Actions workflow below for more details. You can also ping conda-forge/core (using the @ notation) for further assistance or you can try rerendering locally.

The following suggestions might help debug any issues:

  • Is the recipe/{meta.yaml,recipe.yaml} file valid?
  • If there is a recipe/conda-build-config.yaml file in the feedstock make sure that it is compatible with the current global pinnnings.
  • Is the fork used for this PR on an organization or user GitHub account? Automated rerendering via the webservices admin bot only works for user GitHub accounts.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/13015053431. Examine the logs at this URL for more detail.

@conda-forge-admin
Copy link
Contributor

conda-forge-admin commented Jan 28, 2025

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 usually follow the syntax in our documentation for specifying the Python version.
    • For the test.requires section of <output 1 output, you should usually use python {{ python_min }} for the python entry.
    • For the test.requires section of <output 2 output, you should usually use python {{ python_min }} for the python entry.
    • If the package requires a newer Python version than the currently supported minimum version on conda-forge, you can override the python_min variable by adding a Jinja2 set statement at the top of your recipe (or using an equivalent context variable for v1 recipes).
  • ℹ️ The recipe is not parsable by parser conda-souschef (grayskull). This parser is not currently used by conda-forge, but may be in the future. We are collecting information to see which recipes are compatible with grayskull.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/13016662462. Examine the logs at this URL for more detail.

@beckermr
Copy link
Member

@conda-forge-admin rerender

@beckermr
Copy link
Member

I am going to merge this one @conda-forge/basemap. I'll clean up any issues if we find them later.

@beckermr beckermr merged commit 58ae8d6 into conda-forge:main Jan 28, 2025
24 checks passed
@regro-cf-autotick-bot regro-cf-autotick-bot deleted the noarch_python_min-migration-1_h93ea52 branch January 28, 2025 18:22
@molinav
Copy link
Member

molinav commented Jan 29, 2025

@beckermr This PR has introduced some unexpected behaviour in the basemap recipe. This recipe is a bit special, because it does not only build basemap (Python library with compiled extensions), but also the auxiliary basemap-data and basemap-data-hires (Python pure libraries, and indeed they only contain support data).

The basemap-data and basemap-data-hires subrecipes should only generate a single conda package that is usable across all the supported Python versions. When this is correct in the recipe, the generated conda package hash should be the same across Python versions, and then it is redundant to build it for the whole build matrix (this is the reason for the conditional block that was removed by this PR in 8643797).

The previous recipe would always generate basemap-data and basemap-data-hires with names noarch/basemap-data-1.3.2-pyhd8ed1ab_X.tar.bz2 and noarch/basemap-data-hires-1.3.2-pyhd8ed1ab_X.tar.bz2, respectively, with X being the build number for the data packages. Indeed, the usual changes in the conda recipe that require a build number update only play a role in basemap and not in basemap-data and basemap-data-hires, so the build number for the data packages should stay as it is, as long as nothing changes for them (e.g. they are unaffected by GEOS upgrades, NumPy upgrades, Python range updates).

With the removal of the conditional block in 8643797 plus additional changes in the recipe by the PR (e.g. replacing pin_compatible with pin_subpackage for the data packages done in f8e49ee), the hash for the data packages is not a single one anymore, and the recipe generates different data packages for the elements of the build matrix, see e.g. here:

https://anaconda.org/conda-forge/basemap-data/files

  | conda | 24.1 MB | \| noarch/basemap-data-1.3.2-pyh57928b3_2.conda | 12 hours and 45 minutes ago | 28 | main
  | conda | 24.0 MB | \| noarch/basemap-data-1.3.2-pyh37bb5a9_2.conda | 12 hours and 48 minutes ago | 25 | main
  | conda | 24.3 MB | \| noarch/basemap-data-1.3.2-pyh298c13c_2.conda | 12 hours and 49 minutes ago | 27 | main
  | conda | 24.5 MB | \| noarch/basemap-data-1.3.2-pyhf450f58_2.conda | 12 hours and 49 minutes ago | 23 | main
  | conda | 24.0 MB | \| noarch/basemap-data-1.3.2-pyh694c41f_2.conda | 12 hours and 50 minutes ago | 27 | main
  | conda | 23.9 MB | \| noarch/basemap-data-1.3.2-pyh57928b3_1.conda | 14 hours and 33 minutes ago | 24 | main
  | conda | 24.2 MB | \| noarch/basemap-data-1.3.2-pyh37bb5a9_1.conda | 14 hours and 38 minutes ago | 33 | main
  | conda | 23.9 MB | \| noarch/basemap-data-1.3.2-pyh298c13c_1.conda | 14 hours and 38 minutes ago | 28 | main
  | conda | 24.0 MB | \| noarch/basemap-data-1.3.2-pyhf450f58_1.conda | 14 hours and 39 minutes ago | 27 | main
  | conda | 24.1 MB | \| noarch/basemap-data-1.3.2-pyh694c41f_1.conda | 14 hours and 40 minutes ago | 27 | main

https://anaconda.org/conda-forge/basemap-data-hires/files

  | conda | 80.7 MB | \| noarch/basemap-data-hires-1.3.2-pyh57928b3_2.conda | 13 hours and 1 minute ago   | 22 | main
  | conda | 80.3 MB | \| noarch/basemap-data-hires-1.3.2-pyh37bb5a9_2.conda | 13 hours and 4 minutes ago  | 22 | main
  | conda | 80.4 MB | \| noarch/basemap-data-hires-1.3.2-pyhf450f58_2.conda | 13 hours and 5 minutes ago  | 19 | main
  | conda | 80.3 MB | \| noarch/basemap-data-hires-1.3.2-pyh298c13c_2.conda | 13 hours and 5 minutes ago  | 24 | main
  | conda | 80.2 MB | \| noarch/basemap-data-hires-1.3.2-pyh694c41f_2.conda | 13 hours and 6 minutes ago  | 22 | main
  | conda | 79.8 MB | \| noarch/basemap-data-hires-1.3.2-pyh57928b3_1.conda | 14 hours and 48 minutes ago | 28 | main
  | conda | 80.6 MB | \| noarch/basemap-data-hires-1.3.2-pyh37bb5a9_1.conda | 14 hours and 54 minutes ago | 24 | main
  | conda | 80.4 MB | \| noarch/basemap-data-hires-1.3.2-pyh298c13c_1.conda | 14 hours and 54 minutes ago | 27 | main
  | conda | 79.8 MB | \| noarch/basemap-data-hires-1.3.2-pyhf450f58_1.conda | 14 hours and 55 minutes ago | 21 | main
  | conda | 80.3 MB | \| noarch/basemap-data-hires-1.3.2-pyh694c41f_1.conda | 14 hours and 56 minutes ago | 23 | main

So we have the same data packages pushed several times to the conda registry, mainly because the hash has changed but not the actual data package contents. I find this important, since e.g. basemap-data-hires has a size of about 80 MB. basemap-data-hires did not need any rebuild, but the new recipe now generates five copies of basemap-data-hires instead of one, and they are regenerated whenever the data build number is increased: this translates into pushing 5x80 MB = 400 MB to the conda registry "for free".

@beckermr
Copy link
Member

Yes I was aware this would push more than one copy of the package. The previous recipe had two significant issues.

  1. If the version of the data packages ever changed, the builds on all platforms but linux would fail.
  2. The version constraint on the data packages is pulled from the latest already existing data package. So if the data package version ever changed, you'd get the wrong pin in the recipe.

I think I can push a fix that will solve both of these issues and end up with only one build uploaded.

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