Cleanup _libcall and _usercall distinction #199
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report
@@ Coverage Diff @@
## main #199 +/- ##
==========================================
+ Coverage 81.47% 81.58% +0.11%
==========================================
Files 67 67
Lines 5052 5040 -12
==========================================
- Hits 4116 4112 -4
+ Misses 936 928 -8
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
onnxscript/values.py
Outdated
| @@ -142,6 +142,70 @@ class OnnxClosure: | |||
| function: Any | |||
|
|
|||
|
|
|||
| def adapt_to_eager_mode(inputs): | |||
There was a problem hiding this comment.
Should we hide this function? Same for the other
| def adapt_to_eager_mode(inputs): | |
| def _adapt_to_eager_mode(inputs: np.ndarray | tensor.Tensor | bool | Number | list[Any] | tuple[Any, ...]): |
There was a problem hiding this comment.
For the type, the ideal would be to use a recursive type definition (python/mypy#731) ... do you think we can use it, or is it too experimental still?
There was a problem hiding this comment.
From the comments it's on by default for mypy 0.990. since we are using 0.991 it may be worth a try
| raise TypeError(f"Unexpected input type {type(input)}.") | ||
|
|
||
| result = adapt(inputs) | ||
| return result, has_array |
There was a problem hiding this comment.
optional but I think some tests will be helpful
|
@justinchuby do you know why the lint check is failing? It's complaining about "cls.atol = 1e-7 # type: ignore[attr-defined]", but I didn't touch this line at all ... I guess it must already have been there |
|
I fixed it in the previous PR. Does rebasing help? The issue was because lintrunner previously ignored some mypy errors that it shouldn’t and I fixed it in the package |
onnxscript/values.py
Outdated
|
|
||
| def _adapt_to_eager_mode(inputs): | ||
| UserModeValue = Union[ |
There was a problem hiding this comment.
Quotes are not needed since they are delayed evaluated.
|
I ended up leaving the type-definitions commented out: it is a bit non-trivial to get all typing annotations to work (as other existing code doesn't have necessary type annotations), and I will need to get other things setup to tackle something like this. |
Cleanup _libcall and _usercall distinction