Skip to content

fix import order for torchvision.prototype.datasets #4538

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

Merged
merged 1 commit into from
Oct 5, 2021

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Oct 5, 2021

This was broken by #4384, but not detected since we don't have tests for the prototype functionality yet.


# Load this last, since some parts depend on the above being loaded first
from ._api import register, _list as list, info, load
from ._api import register, _list as list, info, load # usort: skip
Copy link
Member

Choose a reason for hiding this comment

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

Just curious, what's the reason we need to load these after _home?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To avoid cyclic imports. Importing anything from _api here will try to import home:

from torchvision.prototype.datasets import home

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

LGTM when green, thanks

@pmeier pmeier merged commit c3062d2 into pytorch:main Oct 5, 2021
@pmeier pmeier deleted the fix-prototype-imports branch October 5, 2021 09:45
facebook-github-bot pushed a commit that referenced this pull request Oct 7, 2021
Reviewed By: kazhang

Differential Revision: D31391272

fbshipit-source-id: 093631390dcace3d1e3e21d264f68405399b0743
mszhanyi pushed a commit to mszhanyi/vision that referenced this pull request Oct 19, 2021
cyyever pushed a commit to cyyever/vision that referenced this pull request Nov 16, 2021
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.

3 participants