update recipe by wolfv · Pull Request #12 · conda-forge/pysoundfile-feedstock

wolfv · 2020-12-02T08:08:12Z

Checklist

Used a fork of the feedstock to propose changes
Bumped the build number (if the version is unchanged)
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.

conda-forge-linter · 2020-12-02T08:08:18Z

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

I do have some suggestions for making it better though...

For recipe:

License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

conda-forge-linter · 2020-12-02T08:11:04Z

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

wolfv · 2020-12-02T08:13:11Z

@scopatz this fixes the sf_wchar_open not found issue on windows that quite a few users have been complaining here: conda-forge/libsndfile-feedstock#14

The problem is that noarch packages are only compiled on Linux, but the sf_wchar_open is only considered on Windows: https://github.com/bastibe/python-soundfile/blob/5f650bc017176424cb39ffbc544f4d80ca5179e9/soundfile_build.py#L130-L134

Would be awesome if you can merge this :) I also added myself as a maintainer.

mgeier · 2020-12-02T09:14:51Z

@wolfv It's great that you found the root cause!

I don't think it's necessary to make platform-dependent packages, though.

What about just defining PYSOUNDFILE_PLATFORM=win32 during the build?

I guess this wouldn't hurt the other platforms.

UPDATE: I've just tried it on Linux, and the additional definition doesn't cause an error. In the code for opening files, the architecture is checked anyway, so sf_wchar_open() will not be called on Linux anyway.
However, the unneeded helper package _soundfile_data will be created in this case, which is not great.
Probably this can be worked around by monkey-patching sys.platform = 'win32' during installation?

Or can you just patch the offending code in soundfile_build.py?

See conda-forge/pysoundfile-feedstock#12.

wolfv · 2020-12-02T09:35:23Z

@mgeier are you interested in being a co-maintainer?

mgeier · 2020-12-02T09:41:13Z

@mgeier are you interested in being a co-maintainer?

No, thanks, I'm not a conda user.

But I'm interested in making the soundfile package work without hiccups for everyone.

A possible solution from the soundfile side could be: bastibe/python-soundfile#288.
But I don't know (yet) whether this works on all platforms and it wouldn't help in the short term anyway.

conda-forge-linter · 2020-12-02T09:43:29Z

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

I do have some suggestions for making it better though...

For recipe:

noarch: python recipes are recommended to have a lower bound on the python version. This recommendation will become a requirement in the future.

…da-forge-pinning 2020.12.02.08.58.36

wolfv · 2020-12-02T09:45:47Z

@mgeier this is only so that you could press the merge button on this feedstock to release new versions of this software :) We have a bot that comes around to trigger version updates when you make them on PyPI for example.

Thanks for your suggestions, I applied them and tested locally on Linux and it seems to work fine.

scopatz · 2020-12-02T13:43:40Z

 {% set pypi_name = "SoundFile" %}
-{% set version = "0.10.2" %}
-{% set sha256 = "637f6218c867b8cae80f6989634a0813b416b3e6132480d056e6e5a89a921571" %}
+{% set version = "0.10.3.post1" %}


This doesn't seem to be a released version. Why not do 0.10.3 with a patch?

It's the current version on PyPI and Debian has this one as well: https://packages.debian.org/sid/python3-soundfile (0.10.3+post1-1)

OK, maybe this should go onto the rc label then? I don't usually use any post/alpha/betas on the main label?

I don't understand? Why not put a post release on the main channel? Isn't it the equivalent of incrementing the build number in the pypi world? If Debian puts it in the main channel, I think we should, too.
It's the most recent version and was released over a year ago. Time for cf to catch up! :)

Post-releases seem to be for minor, non-code changes so itr should be fine to either pull from the upstream 0.10.3 or rename post releases to the latest main release.

I think it makes sense in general for conda-forge to have its own lifetimes for packages, and not be tied to other package managers.

So just to be clear, I think it is fine to pull the post release, but the post number can either be ignored in the version field or become the build number.

conda officially supports .postX version strings and the convenience is that we don't need to change the PyPI url etc. and autotickbot etc. should continue to work fine: https://github.com/conda/conda/blob/7132ac7a713a6d61d5da317d98cea961af748d75/conda/models/version.py#L65-L66

scopatz · 2020-12-02T13:44:29Z

-  number: 1001
-  script: python setup.py install --single-version-externally-managed --record record.txt
+  script:
+    - export PYSOUNDFILE_PLATFORM=win32


This is weird for non-windows platforms

agreed, but it's necessary so that the noarch package works on all platforms. @mgeier is fixing it upstream. bastibe/python-soundfile#288

This fixes a longstanding issue for Windows users on conda-forge.

Also I tested on Linux and everything seems to work fine.

Ahh great! Maybe it would be easiest to wait for the fix and a full release then? Alternatively, this could use some heavy documentation and instructions on when to remove

Yeah, I can add some comments. But git blame also always works, and I documented pretty clearly what's going on up there in the comments of this PR.

wolfv · 2020-12-02T13:55:12Z

Thanks for the review @scopatz! :)

wolfv · 2020-12-03T07:12:45Z

@scopatz I added lots of comments as to why we export the variable. I hope this is good to go as it will improve the life of cf's Windows users :)

scopatz · 2020-12-03T16:04:42Z

Thanks @wolfv!

pythonic2020 · 2020-12-03T17:52:45Z

After installing

pysoundfile 0.10.3.post1
libsndfile 1.0.30 build h0e60522_1

I can no longer import librosa 0.8.0. See report here.

scopatz · 2020-12-03T18:29:43Z

Please open a new tracking issue

pythonic2020 · 2020-12-04T14:23:54Z

Done. Here it is.

update recipe

6bb9fc4

'spdxify'

3c38a9b

mgeier reviewed Dec 2, 2020

View reviewed changes

Comment thread recipe/meta.yaml Outdated

mgeier added a commit to mgeier/python-soundfile that referenced this pull request Dec 2, 2020

Define sf_wchar_open() on all platforms

0548620

See conda-forge/pysoundfile-feedstock#12.

mgeier mentioned this pull request Dec 2, 2020

Define sf_wchar_open() on all platforms bastibe/python-soundfile#288

Draft

use win32 as platform for pysoundfile, revert to noarch

d3e008f

MNT: Re-rendered with conda-build 3.20.5, conda-smithy 3.8.5, and con…

96e7276

…da-forge-pinning 2020.12.02.08.58.36

add pip

e793346

scopatz reviewed Dec 2, 2020

View reviewed changes

wolfv mentioned this pull request Dec 2, 2020

dependency problems for noarch #9

Closed

add lots of comments why we export PYSOUNDFILE_PLATFORM

3af35d1

wolfv mentioned this pull request Dec 3, 2020

conda librosa 0.8.0: cffi library Attribute Error using librosa.load() for .wav file librosa/librosa#1260

Closed

scopatz merged commit 2efde41 into conda-forge:master Dec 3, 2020

Uh oh!

Conversation

wolfv commented Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

conda-forge-linter commented Dec 2, 2020

Uh oh!

conda-forge-linter commented Dec 2, 2020

Uh oh!

wolfv commented Dec 2, 2020

Uh oh!

mgeier commented Dec 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

wolfv commented Dec 2, 2020

Uh oh!

mgeier commented Dec 2, 2020

Uh oh!

conda-forge-linter commented Dec 2, 2020

Uh oh!

wolfv commented Dec 2, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wolfv commented Dec 2, 2020

Uh oh!

wolfv commented Dec 3, 2020

Uh oh!

scopatz commented Dec 3, 2020

Uh oh!

pythonic2020 commented Dec 3, 2020

Uh oh!

scopatz commented Dec 3, 2020

Uh oh!

pythonic2020 commented Dec 4, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wolfv commented Dec 2, 2020 •

edited

Loading

mgeier commented Dec 2, 2020 •

edited

Loading