[GitHub] [tvm] michalpiszczek commented on pull request #8924: [ONNX] Add OpSet 13 implementation for Hardmax and type annotations

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

michalpiszczek commented on pull request #8924:
URL: https://github.com/apache/tvm/pull/8924#issuecomment-912737921


   Thanks for the comments @AndrewZhaoLuo @jwfromm ! I am happy to take out the typing changes if that's what it takes to get Hardmax in, but I would share a different perspective first based on a previous experience. 
   
   At $prevjob I worked on a large python repo that was also initially untyped (but wanted to eventually be fully-typed) and we found that the best way to do this was for devs to add typings in and around old code as they went, rather than making separate, self-contained efforts to add types. We learned that the latter was difficult to rely upon because, with feature work and other responsibilities, finding dedicated time to go and fix up old files just didn't happen. On the other hand, embracing the first approach ended up creating a culture of leaving files you touch better than you found them amongst developers, which more organically distributed the burden of adding types. Gradually, that code base was able to become fully-typed.
   
   I would suggest we take the same approach here, particularly because TVM's CI duration adds a non-trivial time/attention cost to making any change, further discouraging standalone cleanups. 
   
   Please let me know WYT @AndrewZhaoLuo @jwfromm . Thanks! 🙂 
   
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

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