fix: use custom strtobool if distutils not available #78
Conversation
|
Thanks, but this issue has already been resolved by pinning the Python version, can you reproduce the issue versions > v0.16 ? |
|
|
At some point we will need to change from the old python version, as it is a temporary solution. It is fine to think about how to migrate out. |
| def strtobool(val): | ||
| """ | ||
| Convert a string representation of truth to true (1) or false (0). | ||
|
|
||
| True values are 'y', 'yes', 't', 'true', 'on', and '1'; false values | ||
| are 'n', 'no', 'f', 'false', 'off', and '0'. Raises ValueError if | ||
| 'val' is anything else. | ||
| """ | ||
| val = val.lower() | ||
| if val in ('y', 'yes', 't', 'true', 'on', '1'): | ||
| return 1 | ||
| elif val in ('n', 'no', 'f', 'false', 'off', '0'): | ||
| return 0 | ||
| else: | ||
| raise ValueError("Invalid truth value '{}'".format(val)) |
There was a problem hiding this comment.
(Disclaimer: I'm not a maintainer, just a drive-by reviewer.)
The substance of the solution is good, but the current implementation hurts maintainability: defining a function inline, in a try-catch block, in an import statement, leaves a bit of a mess.
Ideally, we'd define this function in one of the other library files, and add unit tests. But, this project appears to be a single-python-file setup which doesn't have unit tests. So, I think the cleanest thing to do would be to stop trying to import distutils, and just define strtobool as a function in this file.
In fact, since we have total control of this file, and there is only a single usage, we can dispense with the awkward bool(strtobool(...)) at the callsite (line 314 in the "before" version), and simply return bool directly from this function.
inspiration: