Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: replace strtobool for local function #128

Merged
merged 2 commits into from Feb 2, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion decouple.py
Expand Up @@ -5,7 +5,7 @@
from shlex import shlex
from io import open
from collections import OrderedDict
from distutils.util import strtobool
from util import strtobool

# Useful for very coarse version differentiation.
PYVERSION = sys.version_info
Expand Down
21 changes: 21 additions & 0 deletions tests/test_util.py
@@ -0,0 +1,21 @@
import pytest
from util import strtobool
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utils is a symptom of lack of a proper abstraction. Please, do not use it. Keep the strtobool function on the main file decouple.py since it's not used anywhere else.



def test_true_values():
true_list = ["y", "yes", "t", "true", "on", "1"]
for item in true_list:
assert strtobool(item) == 1


def test_false_values():
false_list = ["n", "no", "f", "false", "off", "0"]
for item in false_list:
assert strtobool(item) == 0


def test_invalid_value_text():
invalid_list = ["Invalid_Value_1", "1nv4l1d_V4lu3_2", "Invalid_Value_3"]
for value in invalid_list:
with pytest.raises(ValueError, match="invalid truth value '%s'".format(value)):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I don't like this single test with a loop.
  2. The error string should be a constant without format.

Please check pytest's parametrize

strtobool(value)
9 changes: 9 additions & 0 deletions util.py
@@ -0,0 +1,9 @@
def strtobool(value):
value = value.lower()
if value in ["y", "yes", "t", "true", "on", "1"]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comparisons are O(N) and always recreate the list. Make the list definition global and use a set instead of a list.

result = True
elif value in ["n", "no", "f", "false", "off", "0"]:
result = False
else:
raise ValueError("invalid truth value '%s'".format(value))
return result