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

Remove all uses of assert in the codebase #2590

Merged
merged 2 commits into from Dec 10, 2016
Merged

Remove all uses of assert in the codebase #2590

merged 2 commits into from Dec 10, 2016

Conversation

Kwpolska
Copy link
Member

The assert statement can be disabled (python -O) and often does not
explain what went wrong. This replaces all uses of assert with an if
and a ValueError.

This needs a logic review, I had to invert conditions manually.

Signed-off-by: Chris Warrick kwpolska@gmail.com

The `assert` statement can be disabled (`python -O`) and often does not
explain what went wrong. This replaces all uses of `assert` with an `if`
and a `ValueError`.

Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Copy link
Contributor

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

LGTM (I checked the logic, not the syntax, obviously...)

@@ -66,7 +66,8 @@ def _do_classification(self, site):
for lang in site.config['TRANSLATIONS'].keys():
# Extract classifications for this language
classifications[lang] = taxonomy.classify(post, lang)
assert taxonomy.more_than_one_classifications_per_post or len(classifications[lang]) <= 1
if not taxonomy.more_than_one_classifications_per_post and len(classifications[lang]) > 1:
raise ValueError("Too many {0} classifications for post {1}".format(taxonomy.classification_name, post.source_path)
Copy link
Contributor

Choose a reason for hiding this comment

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

There's one ) missing at the end of the line.

Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@Kwpolska Kwpolska merged commit 64762d7 into master Dec 10, 2016
@Kwpolska Kwpolska deleted the remove-asserts branch December 10, 2016 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants