Fix misleading annotation in the documentation

[xadupre]

The function does not seem to work inplace in all cases.

[codecov]

Codecov Report

Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.76%. Comparing base (288481d) to head (4e35322).

Files with missing lines	Patch %	Lines
onnxscript/tools/benchmark/benchmark_helpers.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2046   +/-   ##
=======================================
  Coverage   71.76%   71.76%           
=======================================
  Files         225      225           
  Lines       29275    29276    +1     
  Branches     3455     3455           
=======================================
+ Hits        21010    21011    +1     
  Misses       7133     7133           
  Partials     1132     1132           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[justinchuby]

We should make this a TypeVar

define TModel as a typevar then

Suggested change

	def optimize(model: ir.Model | onnx.ModelProto, *args, **kwargs) -> ir.Model | onnx.ModelProto:
	def optimize(model: TModel, *args, **kwargs) -> TModel:

etc.

[xadupre]

I just did it for the documentation because without it, I assumed the modifications were done inplace which is not always the case. For that only purpose, I feel like keeping both types gives better information than a TypeVar.

[justinchuby]

Using union confuses the type checker in user code because then it will not know which is the return type without an isinstance assertion. That's going to be an inconvenience to users. Right now since we don't annotate at all, the type checker is able to infer the return type

[gramalingam]

Maybe we need to clean this up. It stems from our transition from proto to IR, which also simultaneously switched from copy-and-optimize to optimize-in-place. One issue to resolve is how much backward-compatibility we need here. Ideally, we should migrate away from proto-based optimization (not sure if there is any usage of this remaining), and have just in-place IR modification.

[justinchuby]

A potential way to this this would be annotating with only IR and hide the usage of the proto input. At some point after some deprecation period we can remove the logic entirely.