-
-
Notifications
You must be signed in to change notification settings - Fork 646
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
Support "extra" dependencies/non-declared transitive dependencies #19672
Labels
Comments
thejcannon
added a commit
that referenced
this issue
Aug 25, 2023
`torch.utils.tensorboard` imports `tensorboard` (or dies trying). The original intent is "if a file imports it, it should receive a dependency on `tensorboard`". Unfortunately the actual behavior is that _only_ tensorboard is inferred (and not `torch`). Worse, an import from `tensorboard` is now ambiguous (because it's missing from the value). So we'll remove it and cherry-pick to 2.17.x (when it was introduced). Later we can fix the underlying issue of being able to model an import mapping to multiple packages. See #19672
huonw
pushed a commit
that referenced
this issue
Aug 26, 2023
`torch.utils.tensorboard` imports `tensorboard` (or dies trying). The original intent is "if a file imports it, it should receive a dependency on `tensorboard`". Unfortunately the actual behavior is that _only_ tensorboard is inferred (and not `torch`). Worse, an import from `tensorboard` is now ambiguous (because it's missing from the value). So we'll remove it and cherry-pick to 2.17.x (when it was introduced). Later we can fix the underlying issue of being able to model an import mapping to multiple packages. See #19672
thejcannon
added a commit
that referenced
this issue
Aug 26, 2023
…19673) (#19677) `torch.utils.tensorboard` imports `tensorboard` (or dies trying). The original intent is "if a file imports it, it should receive a dependency on `tensorboard`". Unfortunately the actual behavior is that _only_ tensorboard is inferred (and not `torch`). Worse, an import from `tensorboard` is now ambiguous (because it's missing from the value). So we'll remove it and cherry-pick to 2.17.x (when it was introduced). Later we can fix the underlying issue of being able to model an import mapping to multiple packages. See #19672 Co-authored-by: Josh Cannon <[email protected]>
github-actions bot
pushed a commit
that referenced
this issue
Aug 29, 2023
`torch.utils.tensorboard` imports `tensorboard` (or dies trying). The original intent is "if a file imports it, it should receive a dependency on `tensorboard`". Unfortunately the actual behavior is that _only_ tensorboard is inferred (and not `torch`). Worse, an import from `tensorboard` is now ambiguous (because it's missing from the value). So we'll remove it and cherry-pick to 2.17.x (when it was introduced). Later we can fix the underlying issue of being able to model an import mapping to multiple packages. See #19672
huonw
pushed a commit
that referenced
this issue
Aug 29, 2023
…19673) (#19699) `torch.utils.tensorboard` imports `tensorboard` (or dies trying). The original intent is "if a file imports it, it should receive a dependency on `tensorboard`". Unfortunately the actual behavior is that _only_ tensorboard is inferred (and not `torch`). Worse, an import from `tensorboard` is now ambiguous (because it's missing from the value). So we'll remove it and cherry-pick to 2.17.x (when it was introduced). Later we can fix the underlying issue of being able to model an import mapping to multiple packages. See #19672 Co-authored-by: Josh Cannon <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is your feature request related to a problem? Please describe.
Some packages have optional dependencies that aren't properly declared in their metadata. For example, the
torch
moduletorch.utils.tensorboard
needstensorboard
. It may also sometimes needPillow
to write images to log files. Similarly, thetransformers
library can usebitsandbytes
for optimizations but does not have any extras or dependencies for it.This leads to issues with dependency inference, since a Pants user would expect all transitive dependencies to be included. The solution for this is to declare explicit dependencies on these packages. However, since
pants
already ships a list for mapping modules to names, it could be interesting to also support (and ship) a module-to-dependencies list in the same way.Describe the solution you'd like
One possible way of declaring this would be a system similar to
module_mappings
.Describe alternatives you've considered
It is possible to solve this with explicit dependencies.
Additional context
This thread on Slack
The text was updated successfully, but these errors were encountered: