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

fix imports and create datachain.torch #60

Merged
merged 3 commits into from
Jul 16, 2024
Merged

fix imports and create datachain.torch #60

merged 3 commits into from
Jul 16, 2024

Conversation

dberenbaum
Copy link
Contributor

Another attempt at fixing #52 and related issues around imports.

  • Moves pillow to core dependencies since it is lightweight and need for basic operations like reading image files
  • Renames rest of cv optional deps to torch since they are all torch-based and their usage is broader than cv
  • Moves all torch code to datachain.torch to simplify imports and unify torch-related code

Copy link

cloudflare-workers-and-pages bot commented Jul 16, 2024

Deploying datachain-documentation with  Cloudflare Pages  Cloudflare Pages

Latest commit: 28072ff
Status: ✅  Deploy successful!
Preview URL: https://aa92c7e6.datachain-documentation.pages.dev
Branch Preview URL: https://torch-imports.datachain-documentation.pages.dev

View logs

@skshetry
Copy link
Member

@dberenbaum, can we make these imports lazy instead?

Copy link

codecov bot commented Jul 16, 2024

The author of this PR, dberenbaum, is not an activated member of this organization on Codecov.
Please activate this user on Codecov to display this PR comment.
Coverage data is still being uploaded to Codecov.io for purposes of overall coverage calculations.
Please don't hesitate to email us at support@codecov.io with any questions.

@dberenbaum
Copy link
Contributor Author

dberenbaum commented Jul 16, 2024

@skshetry Do you mean putting the imports inside each individual function/method? IMHO it's not worth the added complexity, especially since this is already extracted out to datachain.torch, but would be curious to hear from others.

Edit: The only place where any torch functionality is called in the core code is DataChain.to_pytorch(), where it is imported lazily.

@dberenbaum dberenbaum merged commit 9ca80fb into main Jul 16, 2024
18 of 19 checks passed
@dberenbaum dberenbaum deleted the torch_imports branch July 16, 2024 20:44
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