Skip to content

Conversation

@Mistobaan
Copy link
Contributor

@Mistobaan Mistobaan commented Aug 22, 2019

  • explicitly excluding alsa when building on linux
  • torchaudio was using deprecated types and generating warnings

setup.py Outdated

libraries = []
if platform.system() == 'Linux':
libraries = ['asound']
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we adding this library? where is it used?

@Mistobaan
Copy link
Contributor Author

Mistobaan commented Aug 22, 2019 via email

@jamarshon
Copy link
Contributor

I don't think soxlib uses asound. If you look at our packaging build_from_source.sh, those are all the dependencies supported/used by torchaudio for generating our binaries. Perhaps it would be more useful if you create an issue using the issue template with clear steps for reproducing the issue so that we can track it and figure out what is happening

@Mistobaan
Copy link
Contributor Author

Yes. That was it. sox configure was picking up the lib alsa automatically. The CI image does not have the lib so is not being configured. Excplicity Removed.

Explicit is better than implicit.

cuda: 10.0.130
pytorch-git: 8554416a199c4cec01c60c7015d8301d2bb39b64
pyorch:1.2.0

@jamarshon
Copy link
Contributor

jamarshon commented Aug 22, 2019

Yes that makes more sense now 😄. @Mistobaan btw, why are you building packages (i.e. conda (.bz2) or pip (.whl))? There are pre-built ones or you can build from source e.g python setup.py install

@Mistobaan
Copy link
Contributor Author

If I want to contribute to the project. I think is good to be able to build it from scratch :)

@jamarshon
Copy link
Contributor

Hmm, you don't need to build conda/pip binaries to contribute to the project though. Those are more for distribution. You only need need to build from source.

The current packaging scripts are run in a certain environment for the main purpose of generating binaries for release. The scripts are not intended to be the default way for users to install the library

@Mistobaan
Copy link
Contributor Author

I see what you are saying, but I like to run the CI build script on my environment to spot low hanging errors before the CI does.

@Mistobaan
Copy link
Contributor Author

friendly ping. anyone that can review this (and few others) PR?

Copy link
Contributor

@vincentqb vincentqb left a comment

Choose a reason for hiding this comment

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

Updated PR description. This looks good to me. Thanks!

@vincentqb vincentqb merged commit d26e112 into pytorch:master Aug 29, 2019
vincentqb added a commit to vincentqb/audio that referenced this pull request Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants