Conversation
build_wheels.py
Outdated
| @@ -1,23 +1,14 @@ | |||
| import os | |||
There was a problem hiding this comment.
Since this is a script, it should probably be executable and have a shebang?
build_wheels.py
Outdated
| pass | ||
|
|
||
| for platform, archs in architectures.items(): | ||
| def make_wheel(platform, arch, dist): |
There was a problem hiding this comment.
The name make_wheel is not quite accurate, since it's also used to make sdist.
build_wheels.py
Outdated
|
|
||
| for platform, archs in architectures.items(): | ||
| def make_wheel(platform, arch, dist): | ||
| os.system('python setup.py clean --all') |
There was a problem hiding this comment.
os.system() is a bit old-school and could probably be replaced with subprocess.run() https://docs.python.org/3/library/subprocess.html#subprocess.run.
This would also provide a nice way to set the environment with env.
build_wheels.py
Outdated
|
|
||
| for platform, archs in architectures.items(): | ||
| def make_wheel(platform, arch, dist): | ||
| os.system('python setup.py clean --all') |
There was a problem hiding this comment.
Simply using 'python' uses Python 2.7 on my (Debian Linux) system (which isn't really a problem but it was probably not intended).
You should probably use sys.executable instead.
setup.py
Outdated
| exec(line) | ||
| break | ||
| else: | ||
| raise RuntimeException('No version number found') |
build_wheels.py
Outdated
| make_wheel('darwin', '64bit', 'bdist_wheel') | ||
| make_wheel('win32', '32bit', 'bdist_wheel') | ||
| make_wheel('win32', '64bit', 'bdist_wheel') | ||
| make_wheel('', '', 'bdist_wheel') |
There was a problem hiding this comment.
Before running sdist, you should remove the egg-info directory, as I've done in https://github.com/spatialaudio/python-sounddevice/blob/master/make_dist.sh.
|
Cool, this has made the build script a bit cleaner. I've used it to compile 0.10.2 and it seems to work great! |
mgeier
left a comment
mgeier
left a comment
There was a problem hiding this comment.
Thanks, it indeed looks quite streamlined.
| @@ -1,23 +1,20 @@ | |||
| import os | |||
| #!/usr/bin/env python | |||
There was a problem hiding this comment.
subprocess.run() is a Python 3 feature, so this should probably be python3?
In fact, you created a tag I don't see the "sdist" in https://pypi.org/project/SoundFile/#files, but probably that's just because I don't know my way around the new warehouse? What are the plans for the deprecated package https://pypi.org/project/PySoundFile/? |
|
BTW, the wheels don't seem to support PyPy 3.4 and 3.5. Also CPython 3.7 is coming up in June ... |
Haha, that's a funny typo! All kidding aside, we could start thinking about a 1.0.0.
Actually, I forgot to upload that. Thanks for pointing that out. I'll have to create a separate upload script. |
I would advise against that in the current situation.
I don't know if that's really necessary, since AFAIK the currently recommended way to upload stuff is a one-liner anyway: How did you upload? |
2 participants
Referring back to #220, we need to improve our build process.
Right now, we should be building correct cross-platform wheels.
However, the source distribution currently includes all binaries, which is not intended.
Also, there is currently no deprecation notice for PySoundFile.