Skip to content

Conversation

@ailzhang
Copy link
Contributor

fix #20781 #20757
hmm I don't know an easy way to add a test to make sure it runs against a package installed as .egg. But i tested it locally with torchvision.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ailzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@ailzhang ailzhang requested review from ezyang and soumith May 22, 2019 20:37
@ezyang
Copy link
Contributor

ezyang commented May 22, 2019

It looks like the cached_zipfile.close() fix also snuck into this PR

@ezyang ezyang changed the title Workaround python2.7 find_module limitation Workaround python2.7 find_module limitation / explicitly close file May 22, 2019
torch/hub.py Outdated
_remove_if_exists(extracted_repo)
# Unzip the code and rename the base folder
cached_zipfile.extractall(hub_dir)
cached_zipfile.close()
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't do this. ZipFile can be used as a context manager; that will ensure it is closed even if an exception is thrown.

Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

Looks good! But please, use a context manager :)

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ailzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@ailzhang merged this pull request in a16708a.

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.

python2 imp.find_module cannot find .egg packages

5 participants