Improve the free parameter info messages #284
Conversation
With this the `parameter_info()` functions handles more parameter types, deals with more DOPs type and handles complex DOPs (structures) properly. Also, I tried to make the results more consistent, but IMO this is not too important here, because the result of this function is supposed to be informal and for human consumption. Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io> Signed-off-by: Michael Hahn <michael.hahn@mbition.io>
since limits are now complex objects, they cannot directly be used for arithmetics anymore. Signed-off-by: Andreas Lauser <andreas.lauser@mbition.io> Signed-off-by: Michael Hahn <michael.hahn@mbition.io>
| of.write(f": int") | ||
| else: | ||
| of.write(f": <unknown type>") | ||
|
|
There was a problem hiding this comment.
I think it would be better to move this logic to the __str__ of the physical datatype class
There was a problem hiding this comment.
I prefer to leave it here, because str(bdt) would need to include the bit sizes of the numeric types (uint32, float64, etc.) which are not really relevant here. (arguably they are even wrong in this context because most DOPs specify a smaller bit size than what the base type is capable of.)
| ul_str = "inf)" | ||
| else: | ||
| ul_delim = ']' if ul.interval_type == IntervalType.CLOSED else ')' | ||
| ul_delim = ')' if ul.interval_type == IntervalType.OPEN else ']' |
There was a problem hiding this comment.
this logic is better off being in the __str__ of the class
There was a problem hiding this comment.
this does not work because limit objects on their own do not know if they are upper or lower limits...
There was a problem hiding this comment.
Not in the limit class, but in its container
There was a problem hiding this comment.
you mean in CompuMethod? I'd rather leave its __str__() method auto-generated by dataclass. This way it is guaranteed to be complete...
| @@ -305,20 +305,20 @@ def test_free_param_info(self) -> None: | |||
| stdout = StringIO() | |||
| with patch("sys.stdout", stdout): | |||
| request.print_free_parameters_info() | |||
| expected_output = "forward_soberness_check: uint8\nnum_flips: uint8\n" | |||
| expected_output = "forward_soberness_check: uint\nnum_flips: uint\n" | |||
There was a problem hiding this comment.
adding a size lead to quite unexpected results like float16. I think the sizes should be removed here anyway, as this function is concerned with the physical types of the parameters and in the physical space, the bit sizes of the internal representation of an object should not matter (only the valid range does).
|
@kayoub5: Do you still see any issues that should be fixed here? |
With this, the
parameter_info()function handles more parameter types, deals with more DOP types and handles complex DOPs (structures) properly. Also, I tried to make the results more consistent, but IMO this is not too important, because the result of this function is intended to be informal and for human consumption.Besides this I found a fixed a small mistake in the handling of case limits in the multiplexer code.
Andreas Lauser <andreas.lauser@mercedes-benz.com>, on behalf of MBition GmbH.
Provider Information