Conversation
fix lfm2.5 vl template
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
Did some deep dive and I really, really like this solution! |
|
Thanks! I'll be cleaning things up in the next couple of days (aiming to merge next week). I just want to make 100% sure all links stay backwards compatible, but things are much cleaner now than they used to be. Also, cuts down on many dependencies, which is nice. |
| * @param {number|bigint} item The item to search for in the tensor | ||
| * @returns {number} The index of the first occurrence of item in the tensor data. | ||
| */ | ||
| indexOf(item) { |
There was a problem hiding this comment.
Is this intentional?
Tensor is exported from the main entry point. That means users could have used Tensor.indexOf(), removing it could cause errors. Imo at least a breaking change.
There was a problem hiding this comment.
Yeah, this was intentional -- Tensor.indexOf actually hasn't been used anywhere in the library for a very long time and probably should have been removed in 4.0 😅 No external code snippets of ours have ever mentioned it either, and has no analogue really to pytorch tensor classes.
There was a problem hiding this comment.
I see. But I'm still not completely sold on the idea of removing public facing methods in a minor update without prior deprecation warning.
Yes, we dont use it ourselfs. But its possible someones app relies on it and now gets syntax errors.
We can go ahead with it if you think the risk is negligible. But we definitely shouldn't do something like that on a whim.
There was a problem hiding this comment.
But we definitely shouldn't do something like that on a whim.
Oh yes 100% agree -- this is a very minor exception that was an internal method which hasn't been used/exposed for quite some time 😅 So, indeed, the risk is negligible.
opening PR early to get early feedback and to get docs previews