Skip to content
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

BC-Breaking: Remove deprecated load_wav functions from backends #1362

Merged
merged 1 commit into from Mar 5, 2021

Conversation

iseessel
Copy link
Contributor

@iseessel iseessel commented Mar 5, 2021

Testing Steps:

Run entire test suite.

I am getting one failure locally, but it doesn't look related to these changes.

Screen Shot 2021-03-05 at 1 22 47 PM

Seems to have to do with imprecision of floating point arithemetic. Let me know if you want me to relax the threshold slightly -- it may be that the tests do not fail on CI build. (edit: I actually don't see the tests running on checks).

Let me know what you think. Thanks!

Copy link
Collaborator

@mthrok mthrok left a comment

Choose a reason for hiding this comment

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

Hi @iseessel

Thanks for working on this. Did you set the sox command path from the artifact of the build process? This kind of precision mismatch happens when sox command from system package manager (like apt, brew) is used. But anyway I do not think that your change is related to the failure you reported, so it should be fine.

The change looks good. I think CircleCI is hiccuping today. I will come back and see if we can trigger CI.

@iseessel
Copy link
Contributor Author

iseessel commented Mar 5, 2021

@mthrok hmm... I did use the locally built sox as opposed to the package.

@mthrok
Copy link
Collaborator

mthrok commented Mar 5, 2021

@iseessel Can you push something so that web hook is delivered? I cannot find a way to trigger CI.

@iseessel iseessel force-pushed the remove-deprecated-io-functions branch from 525c129 to 415f073 Compare March 5, 2021 20:05
@mthrok
Copy link
Collaborator

mthrok commented Mar 5, 2021

Strange... other PRs are triggering CI properly but not this one.

let me see if closing and reopening helps.

@mthrok mthrok closed this Mar 5, 2021
@mthrok mthrok reopened this Mar 5, 2021
@mthrok
Copy link
Collaborator

mthrok commented Mar 5, 2021

here it goes.

@mthrok
Copy link
Collaborator

mthrok commented Mar 5, 2021

@iseessel flake8 is failing. Can you remove the no longer used import statement?

torchaudio/backend/no_backend.py:2:1: F401 'typing.Any' imported but unused

@iseessel
Copy link
Contributor Author

iseessel commented Mar 5, 2021

Yep, I see a few failures, I'll work through them. @mthrok

@iseessel iseessel force-pushed the remove-deprecated-io-functions branch from 415f073 to 84d3c88 Compare March 5, 2021 20:58
@mthrok
Copy link
Collaborator

mthrok commented Mar 5, 2021

@iseessel other PRs landed on master is causing conflict. Could you resolve it?

@iseessel iseessel force-pushed the remove-deprecated-io-functions branch from 84d3c88 to d3f6916 Compare March 5, 2021 21:50
@iseessel
Copy link
Contributor Author

iseessel commented Mar 5, 2021

I've rebased and resolved conflicts. @mthrok

Errors I've fixed:

  1. Sphinx Doc creation was failing -- I've updated relevant rst's to remove deleted methods from the docs.
  2. "torchaudio/backend/no_backend.py:2:1: F401 'typing.Any' imported but unused" style guide was failing. I've removed the import.

Tests Still failing (Look unrelated to PR):

  1. https://github.com/pytorch/audio/pull/1362/checks?check_run_id=204296817. Looks like a dependency issue.

@mthrok mthrok changed the title Remove deprecated #load_wav functions from io backend classes Remove deprecated load_wav functions from backends Mar 5, 2021
@mthrok mthrok merged commit 436470c into pytorch:master Mar 5, 2021
@mthrok
Copy link
Collaborator

mthrok commented Mar 5, 2021

Thanks!

@mthrok mthrok changed the title Remove deprecated load_wav functions from backends BC-Breaking: Remove deprecated load_wav functions from backends Mar 31, 2021
mthrok pushed a commit to mthrok/audio that referenced this pull request Dec 13, 2022
Co-authored-by: holly1238 <77758406+holly1238@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants