od: Extend physical conversion to integer factors, fix typing

[acolomb]

The Variable.phys property docstring mentions "Non integers will be
passed as is." That is correct, but not reflected in the type hints
for the associated ODVariable.encode_phys() / .decode_phys()
functions. Extend the return / argument type to all possible internal
data types, instead of plain int.

Try factor conversion on all NUMBER_TYPES, where it was previously
limited to INTEGER_TYPES. Only apply rounding in encode_phys() if the
variable's data type is actually integer-based.

Allow integer factors in type hint for ODVariable.factor. There is
nothing wrong with using integer factors to yield integers
when decoding. For encoding, the division may still result in a
float, while the rounding behavior is kept unchanged for integer-typed
variables.

Thus, try to parse integer factors as integers from EDS files, not forcing
to a float.

Related comment first given here: #651 (comment)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[bizfsc]

Changed to zero runtime cost and without assert.
Pylance / mypy happy.

[bizfsc]

Suggested change

	pass
	logger.warning(
	"Could not parse Factor for %s in section [%s], ignoring",
	var.name, section,
	)

Yes, we don't have this in many places, but lets get this started. Ar least in debugging the devs can see now, that the parsing failed.

[acolomb]

I have a separate PR pending which cleans up the logging / error behaviour. Including a "strict" mode which raises instead of logging. Just wanted to finish up smaller steps first, so let's keep it out of this one.

[bizfsc]

Suggested change

assert isinstance(value, (int, float))

decode_raw ensure correct typing. no assert in production code. see bigger comment for both functions

[bizfsc]

    def decode_phys(
        self, value: Union[int, bool, float, str, bytes]
    ) -> Union[int, bool, float, str, bytes]:
        if self.data_type in NUMBER_TYPES:
            numeric = cast(Union[int, float], value)
            value = numeric * self.factor
        return value

    def encode_phys(
        self, value: Union[int, bool, float, str, bytes]
    ) -> Union[int, bool, float, str, bytes]:
        if self.data_type in NUMBER_TYPES:
            numeric = cast(Union[int, float], value)
            if self.factor != 1:
                numeric = numeric / self.factor
            if self.data_type in INTEGER_TYPES:
                numeric = round(numeric)
            value = numeric
        return value

Used too much brain power to solve this issue for the current temperatures. But here is the dilemma:

1. don't use assert in production
2. don't change signature
3. don't ask for permission, ask for forgiveness

=> cast is noop in runtime = zero loss and the linter is happy, because the can not see that only int or float can be passed. This should be the best solution. Also numeric / self.factor will raise natural type error for wrong types

[acolomb]

don't use assert in production

Why? I feel like it's the perfect solution.

"In production" means that one should enable optimizations I guess, so the assertions become no-ops. A quick AI summary (without even hinting at type checks) cooked up these three use cases as OK to use assert:

• Internal consistency checks: assert tree.root is not None
• Conditions that indicate programmer bugs: assert isinstance(ast_node, Expression) (Yay!)
• Debugging assumptions: assert cache_entry.owner is self

I'd rather reach a consensus here, before applying suggestions to the PR which are not clearly fixes for an oversight. 😉