Can we get rid of assert to make process not fail?
chenyin0126 opened this issue · 2 comments
There are a lot of assert keywords at repo and they make the process exit and raise an exception. I have a PDX file with one of data-object-prop has value "0.01" and type as "A_UINT32". It goes through this code
def parse_int(value: str) -> int: try: return int(value) except ValueError: v = float(value) assert v.is_integer() return int(v)
since the value is "0.01" and it raises an exception when exec " assert v.is_integer()". I agree we should do validation, but is it worth making the whole flow fail?
Maybe we can do some changes. Here are my thoughts:
Option 1: Instead of using assert, we can use the if statement to check if the value type is expected. If not expected, we can add an error description returned.
Option 2: Instead of using assert, we use an if statement, if the value is not expected, we should ignore this part of the information.
the idea is that if one of these asserts triggers, the input file is incorrect and thus your workflow would most likely break anyway, but later and in much more subtle ways. (the issue you mention above is a good example of this, IMO. also, if you currently would remove all asserts, you would get a lot of mypy errors.) That said, there might be a few incorrect assert statements (if you find any, please open a PR), and the asserts should probably not raise AssertationError, but OdxError and use the use warnings module to make it suppressible.
I'm aware of these problems and -- if nobody beats me to this -- intend to fix them at some point, but IMO there currently are plenty of more important construction sites in the code base.