-
Notifications
You must be signed in to change notification settings - Fork 15
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
Update conda recipe to be more like conda-forge feedstock #661
Conversation
TestingI did a test build of |
@@ -11,24 +12,24 @@ source: | |||
|
|||
build: | |||
number: 0 | |||
script: "{{ PYTHON }} -m pip install . --no-deps -vv" | |||
script: {{ PYTHON }} -m pip install . --no-deps --no-build-isolation -vv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is best to not allow pip
to install missing build dependencies from PyPI (which is what --no-build-isolation
does).
The quotes are not needed.
entry_points: | ||
- zppy = zppy.__main__:main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Entry points no longer need to be defined in conda recipes and removing them removes one more maintenance step we would otherwise have to do.
|
||
requirements: | ||
host: | ||
- python >=3.9 | ||
- python {{ python_min }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and below, conda-forge has moved to defining python_min
(3.9
by default) and ensuring that the package bets built and tested with this minimum version. The idea is that thing tend to break more often when we only test with the newest python than when we only test with the oldest. See https://github.com/conda-forge/cfep/blob/main/cfep-25.md for details.
- pip | ||
- setuptools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zppy
gets installed with setuptools
so this is a necessary dependency. It used to get bundled with pip
but that is no longer the case.
@@ -1,5 +1,6 @@ | |||
{% set name = "zppy" %} | |||
{% set version = "3.0.0rc1" %} | |||
{% set python_min = "3.9" %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be defined here but is set automatically as part of the infrastructure on conda-forge feedstocks.
@xylar Thanks for working on this!
Great, I see 3.0.0rc1 on https://anaconda.org/conda-forge/zppy/files. Was that created using this code, or do I need to merge this and make a Also, perhaps I should have made the PR template clearer (#658), but my intention was if it was a small change, one could likely ignore most of the checkboxes (the numbered sections are sub-headings of "Big Change") |
No, neither. This change is just for convenience and will not be tested anywhere. I would suggest you merge it before we make rc2 (assuming we do) but no additional work or testing is required. |
Oh, that definitely wasn't clear to me. I would suggest adding a comment to that effect to the template. But, yes, that makes a lot more sense now! |
Issue resolution
Select one: This pull request is...
Please fill out either the "Small Change" or "Big Change" section, and delete the other.
Small Change