- 
                Notifications
    You must be signed in to change notification settings 
- Fork 151
pyproject: Use pyproject.toml for building wheels #560
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
Conversation
| Hello @abelgladstone! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: 
 
 Comment last updated at 2024-10-11 13:58:22 UTC | 
| Thanks for this PR! I like this approach in general, but it seems that the "cross-platform package creation" doesn't work anymore. python-sounddevice/.github/workflows/publish.yml Lines 18 to 19 in 2748f70 
 | 
| @mgeier thank you for the reply. I will try make an effort to get the platform specific builds to work as well. Will get back to you soon. | 
ad2d4ab    to
    3239885      
    Compare
  
    | 
 The make_dist script works as expected. As a consequence of intercepting the filename. I had to move the version to a separate VERSION file. So that I can read it without any fuss. I hope that this is acceptable. BTW Thank you for a wonderful little library. I cannot tell you how much I use this. | 
| Thanks for the quick update! I need some more time to review and test this thoroughly, but in the meantime I was wondering whether we should enable building the wheels on PRs: #561. Because otherwise we don't know whether this PR really works, right? | 
| return f"sounddevice-{version}-py3-none-{oses}.whl" | ||
|  | ||
|  | ||
| def prepare_meta_for_build_wheel(config_settings=None): | 
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.
I guess this should be ...
| def prepare_meta_for_build_wheel(config_settings=None): | |
| def prepare_metadata_for_build_wheel(config_settings=None): | 
... but the whole function seems unnecessary, since it is imported already?
| Agreed. | 
| I've merged #561, can you please rebase this PR? | 
3239885    to
    c8b2cc7      
    Compare
  
    | Done | 
| Thanks! The wheels are available here: https://github.com/spatialaudio/python-sounddevice/actions/runs/11293700433?pr=560 Now all of them contain all binaries, but each should only contain the binaries for their own platform. | 
| [tool.setuptools.package-data] | ||
| "_sounddevice_data"= [ | ||
| "portaudio-binaries/*.dll", | ||
| "portaudio-binaries/*.dylib", | 
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.
I guess this causes all wheels to contain all binaries. This should be limited.
Also, the sdist now also contains the binaries, but it shouldn't.
| Thanks for the review. I'll get back to you soon. | 
| After many tries I am unable to find a way to build the project without the setup.py file in which case what is in the project is already good enough IMO. | 
| Thanks a lot for your efforts anyway! AFAIU, the problem is that  Maybe future changes to  If anyone has an idea, please speak up! | 
| I'm closing this for now, but if anyone has an idea how to make this work, please let me know! | 
Remove setup.py and move to pyproject.toml