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

Merged
merged 8 commits into from
Nov 21, 2024

Conversation

matthewfeickert
Copy link
Member

@matthewfeickert matthewfeickert commented Nov 18, 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.

@matthewfeickert matthewfeickert self-assigned this Nov 18, 2024
@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.

@matthewfeickert matthewfeickert marked this pull request as ready for review November 18, 2024 17:28
Copy link
Member Author

@matthewfeickert matthewfeickert left a comment

Choose a reason for hiding this comment

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

@chrisburr as this is noarch why are there multiple Python builds happening and not just one?

@matthewfeickert matthewfeickert force-pushed the mnt/use-noarch branch 3 times, most recently from 6e5187f to 47a011d Compare November 21, 2024 17:38
@matthewfeickert
Copy link
Member Author

@conda-forge-admin, please 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/11958685116. Examine the logs at this URL for more detail.

matthewfeickert and others added 2 commits November 21, 2024 10:49
* 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
* Use a Jinja2 set statement for python_min to keep all the build metadata
  contained in the recipe/meta.yaml and override the global python_min
  with hist's python_min of 3.8.
* Use '{{ PYTHON }}' Jinja2 variable.
* Add '--no-build-isolation' pip install flag given multiple outputs.
* Bump build number.
recipe/meta.yaml Outdated
outputs:
- name: hist-base
build:
noarch: python
entry_points:
- hist=hist.classichist:main
script: python -m pip install . --no-deps -vv
# c.f. https://github.com/conda/grayskull/issues/582#issuecomment-2482370940
script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
Copy link
Member Author

Choose a reason for hiding this comment

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

Hm. I guess {{ PYTHON }} won't work here, even though this is noarch: python. Maybe it is related to why there are multiple Python builds?

@matthewfeickert
Copy link
Member Author

@conda-forge-admin, please rerender

@matthewfeickert matthewfeickert marked this pull request as draft November 21, 2024 17:57
@matthewfeickert
Copy link
Member Author

I'm realizing now that this feedstock doesn't behave like noarch: python at all, and so this PR doesn't have the intended effect. We need to figure out why it isn't acting like noarch before we can merge.

@matthewfeickert
Copy link
Member Author

@conda-forge-admin, please rerender

@matthewfeickert matthewfeickert marked this pull request as ready for review November 21, 2024 20:59
@matthewfeickert matthewfeickert merged commit ae9dcc3 into conda-forge:main Nov 21, 2024
4 checks passed
@matthewfeickert matthewfeickert deleted the mnt/use-noarch branch November 21, 2024 21:02
@matthewfeickert
Copy link
Member Author

Thanks for the useful advice on the top level build map, @chrisburr!

@matthewfeickert matthewfeickert mentioned this pull request Nov 22, 2024
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.

2 participants