Cross-posting here for visibility... Just tried 2....
# developmentg
gorgeous-winter-99296
08/25/2023, 5:29 PMCross-posting here for visibility... Just tried 2.17.0rc4 after the announcement and found a regression in dependency inference that makes one of our tests break. The only way to get the proper dependencies is to add an explicit dependency on torch. With no dependency, we get
tensorboardtorchimport torchtensorboardgorgeous-winter-99296
08/25/2023, 5:31 PMThis occurs both with/without rust parser. I've nuked cache and regen'd lockfiles.
-ltracegorgeous-winter-99296
08/25/2023, 5:46 PM@bitter-ability-32190 https://github.com/tgolsson/pants-repros/tree/main/dep-inference-217
👀 1
b
bitter-ability-32190
08/25/2023, 5:56 PMOK first thought is it certainly is dep inference.
pants dependencies ...bitter-ability-32190
08/25/2023, 5:59 PMbitter-ability-32190
08/25/2023, 6:01 PMSo there's two bugs here.
I think that module mapping ought to have
tensorboardfrom tensorboard.backend.event_processing import event_accumulatortensorboardtorchg
gorgeous-winter-99296
08/25/2023, 6:01 PMThat is just super weird... I think the intention is to make "import tensorboard" source it from "torch.utils.tensorboard", but those are not at all the same...
gorgeous-winter-99296
08/25/2023, 6:02 PMActually, that still doesn't make sense.
gorgeous-winter-99296
08/25/2023, 6:07 PMI'm at a loss for the meaning of it. Tensorboard definitely doesn't put any code in a
torchb
bitter-ability-32190
08/25/2023, 6:07 PMI think you're right. I think the intent is for
torch.util.tensorboardtensorboardbitter-ability-32190
08/25/2023, 6:07 PMBut Pants can't model that this way 😕
g
gorgeous-winter-99296
08/25/2023, 6:07 PMOh, yeah. That makes sense. torch doesn't declare that dependency because it's a "plugin"
b
bitter-ability-32190
08/25/2023, 6:07 PMI guess we could allow a module to come from multiple packages... 🤔
bitter-ability-32190
08/25/2023, 6:08 PMOK mind whiopping up an issue? For 2.17.x I'll whip up a PR removing the line
g
gorgeous-winter-99296
08/25/2023, 6:08 PMBut it isn't true. I think having a way to model "extra-extras" in some way would make more sense.
gorgeous-winter-99296
08/25/2023, 6:08 PMWhat's the issue for if you're removing the line? 😛
b
bitter-ability-32190
08/25/2023, 6:08 PMThe allowing-multiple-packages-per-import
bitter-ability-32190
08/25/2023, 6:09 PMAnd highlighting this issue
g
gorgeous-winter-99296
08/25/2023, 6:09 PMHmm. So "this import implies this dependency"?
b
bitter-ability-32190
08/25/2023, 6:09 PMthese dependencies
bitter-ability-32190
08/25/2023, 6:09 PMplural 🙂
bitter-ability-32190
08/25/2023, 6:09 PMAn import of
torch.utils.tensorboardtorchtensorboardg
gorgeous-winter-99296
08/25/2023, 6:10 PMGotcha. Funnily enough I have a patch in the original code that I've extract here for the same behaviour with Pillow, because torch also doesn't declare that it depends on that...
b
bitter-ability-32190
08/25/2023, 6:10 PMInclude that in the ticket 🙂 We can fix both. And if there's any others you know 🙂
bitter-ability-32190
08/25/2023, 6:13 PMLMK when you have the ticket. I'll push PR
g
gorgeous-winter-99296
08/25/2023, 6:19 PMgorgeous-winter-99296
08/25/2023, 6:26 PMSo I think this would have worked correctly if I didn't import tensorboard directly and imported other torch modules.
b
bitter-ability-32190
08/25/2023, 6:28 PMWell I'm glad you did 😅
g
gorgeous-winter-99296
08/25/2023, 6:30 PMHaha, yeah. Just an extremely specific case... 99% of users would never hit this. I'm reading
tensorboardtensorboard2 Views