Implement Tensor.type

[k223kim]

Before submitting

• Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure to update the docs?
• Did you write any new necessary tests?

What does this PR do?

Fixes #177. At the moment, I did not implement non_blocking=True. Any suggestions would be appreciated! Also, the current CI test that fails seems irrelevant to this PR.

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

As always 🚀

[k223kim]

Hey @mruberry, would you be able to check this draft out? Would appreciate it :)

[mruberry]

Cool stuff, @k223kim! I left some comments for you review. These old torch typestrings are a pain, right? I'm glad torch moved away from them.

I look forward to hearing your thoughts!

[k223kim]

Hi @mruberry thank you for carefully going through my code despite the fact that it may not be needed at all. I should have discussed this earlier before implementing it, but again, thanks for the detailed comments.
I do agree that handling strings is a pain and is not good coding (as pr @t-vi ). Let me remove the current implementation and throw an NotImplementedError for cases where dtype is string. I guess now, I am really questioning if this implementation is needed at all. Cause, if we don't accept strings, how is it different from to? 🤔

[mruberry]

Hi @mruberry thank you for carefully going through my code despite the fact that it may not be needed at all. I should have discussed this earlier before implementing it, but again, thanks for the detailed comments. I do agree that handling strings is a pain and is not good coding (as pr @t-vi ). Let me remove the current implementation and throw an NotImplementedError for cases where dtype is string. I guess now, I am really questioning if this implementation is needed at all. Cause, if we don't accept strings, how is it different from to? 🤔

I think you pointed out that the particular use case we're interested in DOES use strings, and this PR already does a lot of the work, so maybe let's keep them? Just because something is rare doesn't mean it never happens

[k223kim]

Hey @mruberry! I have updated this PR based on your comments :) Although I am questioning if the string support is needed or not, I have included them. Feel free to drop it if that is not necessary. Would appreciate if you can check this out!

[mruberry]

Hey @k223kim!

Cool stuff! I made some additional comments for refinements. I'm curious how a couple things work currently. Maybe there's an issue with testing? Check it out and let me know!

[k223kim]

Hi @mruberry! Thanks so much for the review! I have answered some comments and updated the code accordingly. Please let me know if there is a better way to do this :) Appreciate your help as always!

[k223kim]

Hey @mruberry, thanks for the great explanations and my apologies for the confusion. Hope my code is now easier to understand and maintain :) Would appreciate if you can take a look! 🚀

[mruberry]

Nice work with a tricky operator, @k223kim!

@Borda or @t-vi may need to merge this

[mruberry]

ping @t-vi

[t-vi]

Thank you @k223kim @mruberry