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

Implement TryFrom<Tensor> for primitive numeric types to avoid panic #602

Closed
tqwewe opened this issue Jan 3, 2023 · 4 comments
Closed

Comments

@tqwewe
Copy link

tqwewe commented Jan 3, 2023

Tensor currently implements From<i32>, From<i64>, From<f32>
Tensor currently implements From<Tensor> for: i32, i64, f32, ... for some common rust types, and panics if theres more than one item in the Tensor.

TryFrom trait for these types are not implemented, but would be very handy in cases where you care about handling the error.

@LaurentMazare
Copy link
Owner

Just to be sure, do you actually mean From<i32> i.e. converting an i32 to a Tensor or From<Tensor> for i32 i.e. converting a Tensor to an i32? My guess is that it's the latter as you mention this erroring when there are more than one item in the tensor.

@tqwewe
Copy link
Author

tqwewe commented Jan 4, 2023

Ah I'm sorry, you're right, I mean From<Tensor> and TryFrom<Tensor> for i32, i64, f32, etc.

@tqwewe tqwewe changed the title Implement TryFrom for Tensor Implement TryFrom<Tensor> for primitive numeric types to avoid panic Jan 4, 2023
@LaurentMazare
Copy link
Owner

I've started looking into this but because of specialization not being handled properly, it's tricky to have both the From and TryFrom traits available (more details here). That said, I feel that switching to the TryFrom version might be a good idea although it's a breaking change, and in general the operation should be more fallible by default rather than the other way around. See e.g. #395, it's probably worth thinking a bit more about it (and keen to hear if other people have thoughts) but the idea would be to first switch the From conversion to TryFrom both for scalar and slices, see #605 that starts doing this.

@LaurentMazare
Copy link
Owner

The change has been merged so closing this for now, feel free to re-open if further issues.

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

No branches or pull requests

2 participants