[GitHub] [tvm] t-vi edited a comment on pull request #7023: Save PyTorch frontend state in object

[GitBox <gi...@apache.org>]

t-vi edited a comment on pull request #7023:
URL: https://github.com/apache/tvm/pull/7023#issuecomment-738607046


   Thank you for looking at this.
   
   1. I used static methods to not have unused `self`. I'm not terribly attached to it, if you like the consistency of everything being a regular method.
   2. This is again a consistency thing. Right now all methods apply the operation. Moving some back to returning impl_ has us doing two different things (though I'm not opposed). One alternative I'd consider is to actually register a method for all ops we handle (`convert_op_aten_mm`) and build the conversion dictionary on the fly. For the time being I'll move the things taking extra arguments back to the impl scheme as you suggest.
   3. Yes, I thought I'd do the conversion in pieces. If you prefer I can move these now, too. I will tentatively add them to this PR.
   4. Good point about `_`, in particular as leading `_` in classes is special in Python...
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org