-
-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Adding icepyx package #13866
Adding icepyx package #13866
Conversation
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 ( |
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.
Got hit with a setuptools_scm v0.0.0 version string related issue today at conda-forge/pygmt-feedstock#15, thought I'd save icepyx
from some drama. See also a related PR at conda-forge/pint-feedstock#34.
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.
Let's see if icepyx v0.3.3 from PyPI works
Hmm, that Windows build failure is interesting, probably because of this line that's setting the environment variable in a UNIX-only way? staged-recipes/recipes/icepyx/meta.yaml Line 16 in be41653
I thought that no-arch packages https://conda-forge.org/docs/maintainer/knowledge_base.html#noarch-python are only built on Linux, so not sure what we should do here :/ |
@conda-forge/staged-recipes This is ready for review. Thank you so much for your time! |
noarch: python | ||
script: | ||
- export SETUPTOOLS_SCM_PRETEND_VERSION={{ version }} # [unix] | ||
- set SETUPTOOLS_SCM_PRETEND_VERSION={{ version }} # [win] |
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.
If it is noarch you don't need this. It will failed on the CIs here but there will be no Windows CI in the feedstock. It won't hurt either, but it will never be used.
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.
Okay. So we did do that but the windows CI failed. What you are saying is that would have been fine?
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.
Yep. What @weiji14 said in #13866 (comment) is correct.
There are rare cases of things that pretend to be noarch, passes on Linux, and they we find out that should've been tested on Windows. But if you ever find on of those cases the Windows users will let you know and you'll remove the noarch
in the feedstock later.
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.
Cool. Thanks for that explanation 😄
Overview
This PR adds
icepyx
to conda-forge.Pinging @JessicaS11 so that you're aware. Let me know if there are changes that need to be made in terms of dependencies.
Checklist
url
) rather than a repo (e.g.git_url
) is used in your recipe (see here for more details).