Lensfunpy #10199

mwilson8 · 2019-11-18T20:04:00Z

Checklist

License file is packaged (see here for an example)
Source is from official source
Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged)
If static libraries are linked in, the license of the static library is packaged.
Build number is 0
A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details)
GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there

conda-forge-linter · 2019-11-18T20:04:10Z

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 (recipes/lensfunpy) and found it was in an excellent condition.

mwilson8 · 2019-11-18T20:11:33Z

Getting the error

conda.exceptions.ResolvePackageNotFound: 
  - lensfun==0.3.95=0

@conda-forge/help-c-cpp @conda-forge/help-python-c @conda-forge/staged-recipes I'm not sure the cause for the error, as lensfun is listed as a dependency and is currently in the forge, so I'm unsure of which team to ask

The extra "=0" on the end of the version is what's got me stuck ATM

beckermr · 2019-11-18T20:30:17Z

Frack. This is my fault. The run export in the lensfun package is wrong. I don't get why, but my guess is that the hash depends on the run_export contents which needs the package hash. I will make a PR on the upstream repo to fix. Sit tight!

beckermr · 2019-11-18T20:34:38Z

Mmmmk. Merge this one when it passes and try again.

conda-forge/lensfun-feedstock#1

beckermr · 2019-11-18T21:10:26Z

OK. Wait about 2 hours and then retry this build. It takes a little while for new packages to hit the index.

beckermr · 2019-11-19T01:47:41Z

I will look into this further tonight.

beckermr · 2019-11-19T02:07:34Z

These build errors are not due to the lensfun package missing a run export. It looks like something else. There is a git submodule that is being pulled but failing and a compilation issue as well. Check out the logs https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=95521&view=logs

mbargull · 2019-11-19T07:59:17Z

lensfunpy/_lensfun.c:8158:16: error: too few arguments to function 'lf_lens_interpolate_vignetting'

https://github.com/letmaik/lensfunpy/blob/v1.6.1/lensfunpy/_lensfun.pyx#L164
https://github.com/lensfun/lensfun/blob/v0.3.95/include/lensfun/lensfun.h.in#L1624
lensfun/lensfun@f432766

Breaking changes:

* C interface: all lf_lens_interpolate_...() functions now require an additional crop parameter

=> lensfunpy needs to be updated for the newer lensfun version. (or lensfun<0.3.95 would need to be packaged)

mwilson8 · 2019-11-19T14:10:28Z

@mbargull @beckermr per the issue from the author here

It doesn't seem like lensfunpy will be updated; will work on getting lensfun<0.3.95 made

oblute · 2019-11-19T14:22:41Z

@mbargull @beckermr i'm going to try packaging lensfun v0.3.2 to remedy this issue because as @mwilson8 referenced looks like lensfunpy may not be updated for awhile

beckermr · 2019-11-19T14:25:21Z

Sounds good. You can make a pr against the existing feedstock.

mwilson8 · 2019-11-19T16:58:43Z

@mbargull @beckermr there seems to be a problem in the setup.py on mac, where it's (I'm guessing) trying to rebuild lensfun from scratch??... I'm a little out of my depth on what we could modify in order to have it use conda's dependency management
build

mbargull · 2019-11-19T17:09:48Z

We could patch https://github.com/letmaik/lensfunpy/blob/v1.6.1/setup.py#L222
like so:

-needsCompile = any(s in cmdline for s in ['install', 'bdist', 'build_ext', 'nosetests'])
+needsCompile = False

maybe?

beckermr · 2019-11-19T17:12:15Z

I was in the middle of suggesting a patch when @mbargull beat me to it. Yes this is the right way to go.

beckermr · 2019-11-19T17:12:44Z

See this: https://docs.conda.io/projects/conda-build/en/latest/resources/define-metadata.html#patches

recipes/lensfunpy/meta.yaml

mwilson8 · 2019-11-19T21:39:07Z

@beckermr we're confused why this only fails on osx.. could you take a look when you get a chance

mwilson8 · 2019-11-19T21:55:30Z

Patch the setup.py

This patch seems like it is a little out of my depth.. how much are we needing to change? Could we just remove the win/osx if statement & let all cases use use_pkg_config()?

beckermr · 2019-11-19T21:57:39Z

Probably easiest to just set the value correctly in the if statement as opposed to trying to use pkg_config which may not work. Something like os.path.join(or.environ['PREFIX'], 'include', 'lensfun') is the right value for the include path.

isuruf · 2019-11-19T21:58:23Z

It's good to just remove that condition for osx and let it use pkg-config

isuruf · 2019-11-19T21:58:52Z

You also need to add pkg-config to build requirements.

beckermr · 2019-11-19T21:59:45Z

Ahhh ok. I'm gonna let @isuruf take over here! ;p

mwilson8 · 2019-11-19T22:00:27Z

@isuruf there's a function called use_pkg_config() but you're saying we can use the conda-package pkg-config?

isuruf · 2019-11-19T22:02:53Z

@isuruf there's a function called use_pkg_config() but you're saying we can use the conda-package pkg-config?

No, what I'm saying is for use_pkg_config() python function to work an executable called pkg-config needs to exist. (See https://github.com/letmaik/lensfunpy/blob/master/setup.py#L35-L38) This executable is in pkg-config conda package.

mwilson8 · 2019-11-19T22:33:12Z

@isuruf please review

isuruf · 2019-11-19T22:34:01Z

recipes/lensfunpy/meta.yaml

+  build:
+    - {{ compiler('c') }}
+    - {{ compiler('cxx') }}
+    - lensfun <0.3.95


This should only be in host

isuruf · 2019-11-19T22:34:20Z

recipes/lensfunpy/meta.yaml

+    - python
+    - cython
+    - nose
+    - wheel >=0.25


Why do you need wheel at runtime?

isuruf · 2019-11-19T22:34:40Z

recipes/lensfunpy/meta.yaml

+    - nose
+    - wheel >=0.25
+    - sphinx
+    - sphinx_rtd_theme


sphinx is for docs which is not needed at runtime

isuruf · 2019-11-19T22:34:54Z

recipes/lensfunpy/meta.yaml

+  run:
+    - python
+    - cython
+    - nose


This is needed only for testing. No need at runtime.

isuruf · 2019-11-19T22:35:35Z

recipes/lensfunpy/meta.yaml

+    - pip
+    - python
+    - lensfun <0.3.95
+    - numpy


See https://conda-forge.org/docs/maintainer/knowledge_base.html#building-against-numpy

mwilson8 · 2019-11-20T03:55:07Z

@isuruf please re-review changes

isuruf · 2019-11-20T03:58:49Z

recipes/lensfunpy/meta.yaml

+    - numpy
+    - sphinx
+    - wheel >=0.25
+    - enum34  # [py<34]


Do you need the 3 packages above?

They were listed in a dev-requirements.txt file so we weren't sure

But enum, yes: https://github.com/letmaik/lensfunpy/blob/master/lensfunpy/_lensfun.pyx#L9

That's needed at runtime, so no need for it to be in host. Only run is enough

isuruf · 2019-11-20T04:02:43Z

recipes/lensfunpy/meta.yaml

+    - python
+    - cython
+    - scipy
+    - lensfun <0.3.95


Suggested change

- lensfun <0.3.95

- lensfun <0.3.95

- {{ pin_compatible("lensfun") }}

mwilson8 · 2019-11-20T15:59:55Z

@isuruf ready for review

mwilson8 · 2019-11-20T15:22:13Z

recipes/lensfunpy/meta.yaml

-    - lensfun <0.3.95
+    - {{ pin_compatible('lensfun') }}


@isuruf should I have left lensfun <0.3.95 in there as well as the pin compatible dep? Or is it okay to replace like this

Both are needed IMO.

isuruf · 2019-11-20T18:20:17Z

recipes/lensfunpy/LICENSE

@@ -0,0 +1,21 @@
+The MIT License (MIT)


Can you open an issue upstream about including the license in the sdist?

mwilson8 · 2019-11-20T18:25:55Z

letmaik/lensfunpy#23
But with the author not having commited, commented in over a year the chances are likely low

Mitch Wilson added 3 commits November 18, 2019 14:55

weird dependency error

af131f2

fix commented out code

f15a578

remove glib

3a89c81

beckermr closed this Nov 19, 2019

beckermr reopened this Nov 19, 2019

waiting on pinned lower version of lensfun

87bb368

change to @oblute meta, add patch, remove some deps

2284c3a

beckermr reviewed Nov 19, 2019

View reviewed changes

recipes/lensfunpy/meta.yaml Outdated Show resolved Hide resolved

Mitch Wilson added 5 commits November 19, 2019 14:34

remove exact pin, change to <

4f64354

change download to git, change patch lines

b89a694

back to pypi source

77bab71

possibly had wrong patch lines

37a7a10

use @oblute patch

6ef9171

add c compiler

ace9826

Mitch Wilson added 2 commits November 19, 2019 17:09

attempt new patch

6b76efb

fix patch itself

0c62ddc

isuruf reviewed Nov 19, 2019

View reviewed changes

recipes/lensfunpy/meta.yaml Outdated

- python

- cython

- nose

- wheel >=0.25

Copy link

Member

isuruf Nov 19, 2019

Choose a reason for hiding this comment

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

Why do you need wheel at runtime?

isuruf reviewed Nov 19, 2019

View reviewed changes

recipes/lensfunpy/meta.yaml Outdated

run:

- python

- cython

- nose

Copy link

Member

isuruf Nov 19, 2019

Choose a reason for hiding this comment

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

This is needed only for testing. No need at runtime.

isuruf reviewed Nov 19, 2019

View reviewed changes

Mitch Wilson added 3 commits November 19, 2019 22:20

apply changes

745b449

change numpy pin

10cc111

add enum34

0dc0caa

isuruf reviewed Nov 20, 2019

View reviewed changes

change deps

39d21be

mwilson8 commented Nov 20, 2019

View reviewed changes

change deps & maintainers

150077e

isuruf reviewed Nov 20, 2019

View reviewed changes

isuruf changed the title ~~[weird error] Lensfunpy~~ Lensfunpy Nov 20, 2019

isuruf merged commit fe3d978 into conda-forge:master Nov 20, 2019

mwilson8 deleted the lensfunpy branch November 20, 2019 20:34

	- lensfun <0.3.95
	- lensfun <0.3.95
	- {{ pin_compatible("lensfun") }}

Lensfunpy #10199

Lensfunpy #10199

Conversation

mwilson8 commented Nov 18, 2019

conda-forge-linter commented Nov 18, 2019

mwilson8 commented Nov 18, 2019 • edited Loading

beckermr commented Nov 18, 2019

beckermr commented Nov 18, 2019

beckermr commented Nov 18, 2019

beckermr commented Nov 19, 2019

beckermr commented Nov 19, 2019

mbargull commented Nov 19, 2019

mwilson8 commented Nov 19, 2019 • edited Loading

oblute commented Nov 19, 2019 • edited Loading

beckermr commented Nov 19, 2019

mwilson8 commented Nov 19, 2019

mbargull commented Nov 19, 2019

beckermr commented Nov 19, 2019

beckermr commented Nov 19, 2019

mwilson8 commented Nov 19, 2019

mwilson8 commented Nov 19, 2019

beckermr commented Nov 19, 2019

isuruf commented Nov 19, 2019

isuruf commented Nov 19, 2019

beckermr commented Nov 19, 2019

mwilson8 commented Nov 19, 2019

isuruf commented Nov 19, 2019

mwilson8 commented Nov 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwilson8 commented Nov 20, 2019

Choose a reason for hiding this comment

mwilson8 Nov 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwilson8 commented Nov 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwilson8 commented Nov 20, 2019

mwilson8 commented Nov 18, 2019 •

edited

Loading

mwilson8 commented Nov 19, 2019 •

edited

Loading

oblute commented Nov 19, 2019 •

edited

Loading

mwilson8 Nov 20, 2019 •

edited

Loading