check in linter for required python run-time dependency#547
check in linter for required python run-time dependency#547kain88-de wants to merge 2 commits intoconda-forge:masterfrom
Conversation
kain88-de
force-pushed
the
linter-python-dep
branch
2 times, most recently
from
bfc1b5d to
0a88029
Compare
|
test pass now on travis |
This will ensure that conda is forced to add a python version dependency on the final package.
kain88-de
force-pushed
the
linter-python-dep
branch
from
0a88029 to
f887419
Compare
| for selector_line in selector_lines(fh): | ||
| if 'py3k' in selector_line and 'python' not in requirements_section.get('run', ''): | ||
| lints.append("When building python 2 only packages include python as a run-time " | ||
| "dependency to ensure it can't be installed on python 3.") |
There was a problem hiding this comment.
I have some packages that needs python as a build dependency, but does not require python. I use one python because I don't need a matrix of builds. In that case, according to this, it's a lint failure
There was a problem hiding this comment.
This only ensures a requirement for Python if the package is Python 2 only. could you give an example problem where this would be a problem.
There was a problem hiding this comment.
For example, I have a package with a build script written in python 2. So, I skip py3k, but the package itself doesn't need python in run.
There was a problem hiding this comment.
There was a problem hiding this comment.
Maybe have this just for packages from pypi?
There was a problem hiding this comment.
@kain88-de that assumption is blurred with wheels and we have tons of package that are python but not hosted at PyPI making this solution "flaky." So using the source URL to determined that is not reliable.
TL;DR unless we add an extra metadata to indicate that the recipe is pertinent to a python package we cannot do this.
There was a problem hiding this comment.
OK then I'll drop this in this PR. But I have another question could we have 2 warning types in the linter. The first would be errors that we have now. The second type would be suggestions/hints to help packagers.
There was a problem hiding this comment.
Maybe namespaces could be leveraged depending on if/when/how they are constructed.
There was a problem hiding this comment.
The second type would be suggestions/hints to help packagers.
Indeed. This sort of idea has come up before ( #317 ). Some warnings like, "this might need python", could be a reasonable compromise IMHO.
There was a problem hiding this comment.
OK then I'll drop this in this PR. But I have another question could we have 2 warning types in the linter. The first would be errors that we have now. The second type would be suggestions/hints to help packagers.
Indeed warning type linting is something we should do!
| # 13: If cython is a build-dependency then python should be a run-time | ||
| # dependency. This ensures that packages will be marked correctly by conda | ||
| if 'cython' in requirements_section.get('build', '') and 'python' not in requirements_section.get('run', ''): | ||
| lints.append("When using cython then python should be a run-time dependency") |
There was a problem hiding this comment.
This seems very specific. There are number of errors like this, but we can't add all of them. (swig, numpy as build dependency.)
How about something more generic like if python is an implicit runtime requirement, then it should be added explicitly in runtime requirements?
There was a problem hiding this comment.
How would you detect an implicit requirement?
There was a problem hiding this comment.
Extending this also to numpy/swig/etc isn't hard
|
|
||
| # 13: If cython is a build-dependency then python should be a run-time | ||
| # dependency. This ensures that packages will be marked correctly by conda | ||
| build_deps = ['cython', 'numpy x.x', 'swig'] |
There was a problem hiding this comment.
Should there also be a check that numpy is specified as numpy x.x when it appears in the build requirements?
There was a problem hiding this comment.
swig can be used for lots of things (not just python). Would rather not require people have python with swig.
There was a problem hiding this comment.
I removed swig from that list. Should I add the numpy x.x check?
kain88-de
force-pushed
the
linter-python-dep
branch
4 times, most recently
from
e218798 to
6a02663
Compare
| lints.append("Found python related compilation requirement " | ||
| "please add python as a run-time dependency") | ||
|
|
||
| # 14: If numpy is in build requirements ensure it's stated as `numpy x.x` |
There was a problem hiding this comment.
here is the additional check for numpy x.x in the build requirements. The tests pass now again.
- implicit python dependency when building with cython/numpy
kain88-de
force-pushed
the
linter-python-dep
branch
from
6a02663 to
9dc2e7b
Compare
|
I removed the numpy check again. With conda-forge/conda-forge.github.io#415 it changed anyway how to setup numpy |
|
Is there still any interest in this? |
|
@ocefpaf still interested in lint check? |
|
We are moving away from the I'll leave the decision to you and the conda-smithy authors. |
|
Is there still interest in this? I think updating this to the new default of building against old numpy versions shouldn’t be to difficult. |
|
Is this still interesting or can the PR be closed? |
4 participants
This will ensure that conda is forced to add a python version dependency on the
final package.
I encountered this problem for the MDAnalysis feedstock.
conda-forge/mdanalysis-feedstock#4
I haven't tested this locally yet because I don't know if I can go back to my a stable build or even run the tests without installing the development version.