Commit 067d9f56 authored by Grant Paton-Simpson's avatar Grant Paton-Simpson

Add any / all advisor

parent 361fb59d
Pipeline #566 failed with stages
# https://git.nzoss.org.nz/pyGrant/superhelp
version number: 0.9.5
version number: 0.9.6
author: Grant Paton-Simpson
## Overview
......
......@@ -2,7 +2,7 @@ from setuptools import setup, find_packages # @UnresolvedImport
from codecs import open
from os import path
__version__ = '0.9.5'
__version__ = '0.9.6'
here = path.abspath(path.dirname(__file__))
......
......@@ -643,3 +643,169 @@ def short_circuit(block_dets, *, repeat=False):
conf.MAIN: summary + how2short_circuit + demo,
}
return message
def could_any_or_all(if_el):
could_any = False
could_all = False
boolop_val_els = if_el.xpath('descendant::BoolOp/values')
for boolop_val_el in boolop_val_els:
n_items = len(boolop_val_el.getchildren())
if n_items < conf.MIN4ANY_OR_ALL: ## worth doing any or all
continue
boolop_el = boolop_val_el.getparent()
op_els = boolop_el.xpath('op')
op_el = op_els[0]
op_type_el = op_el.getchildren()[0]
if op_type_el.tag == 'Or':
could_any = True
elif op_type_el.tag == 'And':
could_all = True
if all([could_any, could_all]):
break
return could_any, could_all
@filt_block_advisor(xpath=IF_XPATH)
def any_all(block_dets, *, repeat=False):
"""
Look for cases where using built-in any or all functions makes sense.
"""
if_els = block_dets.element.xpath(IF_XPATH)
could_any_something = False
could_all_something = False
for if_el in if_els:
could_any, could_all = could_any_or_all(if_el)
if could_any:
could_any_something = True
if could_all:
could_all_something = True
if all([could_any_something, could_all_something]): ## LOL - thought I might use 'all' given the context even though only two items
break
if not any([could_any_something, could_all_something]):
return None
if all([could_any_something, could_all_something]):
title_content = "Consider using `any` and `all`"
elif could_any_something:
title_content = "Consider using `any`"
elif could_all_something:
title_content = "Consider using `all`"
else:
raise Exception("Unexpected situation with "
f"could_any_something ({could_any_something}) "
f"and could_all_something ({could_all_something})")
title = layout(f"""\
### {title_content}
""")
if not repeat:
summary = layout("""\
Python has built-in `any` and `all` functions that make your code
more readable when you're evaluating whether all or any of a group
of items are True (or whether _not_ all or _not_ any are True). The
argument has to be an iterable e.g. a list.
""")
demo = (
layout("""\
For example, instead of:
""")
+
layout("""\
if current or valid or safe or permitted:
proceed()
""", is_code=True)
+
layout("""\
you could write the more semantic:
""")
+
layout("""\
if all([current, valid, safe, permitted]):
proceed()
""", is_code=True)
+
layout("""\
or the equivalent, and, arguably, more readable:
""")
+
layout("""\
conditions = [current, valid, safe, permitted]
if all(conditions):
proceed()
""", is_code=True)
+
layout("""\
Or if you wanted the negation the syntax would be:
""")
+
layout("""\
if not all(conditions):
exit()
""", is_code=True)
+
layout("""\
The `any` function works the same way
""")
+
layout("""\
flatmate_positives = [
has_car, has_appliances, has_tv, great_cook, funny]
if any(flatmate_positives):
recruit_to_flat()
""", is_code=True)
)
extra = (
layout("""\
It is easy to forget that an iterable has to be passed in as the
argument rather than the individual items. So:
""")
+
layout("""\
if any(books, magazines, comics, games): ## FAIL TypeError: any() takes exactly one argument (4 given)
relax()
if any([books, magazines, comics, games]): ## SUCCESS
relax()
""", is_code=True)
)
else:
summary = layout("""\
Python has built-in `any` and `all` functions that can make your
code more readable.
""")
demo = ''
extra = ''
message = {
conf.BRIEF: title + summary,
conf.MAIN: title + summary + demo,
conf.EXTRA: extra,
}
return message
......@@ -27,10 +27,12 @@ else:
## When testing user-supplied snippets watch out for the BOM MS inserts via Notepad. AST chokes on it.
TEST_SNIPPET = """\
with open('using_cm.txt') as f:
text = f.read()
f = open('no_cm,txt')
text = f.read()
if apple or banana or cherry or date or elderberry:
pass
if date and elderberry and feijoia:
pass
if (apple or banana or cherry) and (date and elderberry and feijoia):
pass
"""
DEMO_SNIPPET = """\
......@@ -214,6 +216,7 @@ MAX_BRIEF_FUNC_ARGS = 6
MIN_BRIEF_DOCSTRING = 3
MIN_BRIEF_NAME = 3
MAX_BRIEF_NESTED_BLOCK = 20
MIN4ANY_OR_ALL = 3
FUNCTION_LBL = 'function'
METHOD_LBL = 'method'
......
......@@ -16,6 +16,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0,
}
),
(
......@@ -29,6 +30,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0,
}
),
(
......@@ -44,6 +46,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0,
}
),
(
......@@ -61,6 +64,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0,
}
),
(
......@@ -77,6 +81,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0,
}
),
(
......@@ -95,6 +100,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0,
}
),
(
......@@ -120,6 +126,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0,
}
),
(
......@@ -143,6 +150,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0,
}
),
(
......@@ -156,6 +164,7 @@ def test_misc():
ROOT + 'split_group_membership': 1,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 1,
}
),
(
......@@ -169,6 +178,7 @@ def test_misc():
ROOT + 'split_group_membership': 0, ## only dealing with simple cases of all same type and either str or num
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 1,
}
),
(
......@@ -183,6 +193,7 @@ def test_misc():
ROOT + 'split_group_membership': 1,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 1,
}
),
(
......@@ -204,6 +215,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 1, ## per block but no repeats
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0,
}
),
(
......@@ -225,6 +237,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0, ## explicit counts not used as empty/non-empty boolean
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0,
}
),
(
......@@ -243,6 +256,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0,
}
),
(
......@@ -261,6 +275,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0,
}
),
(
......@@ -276,6 +291,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0, ## has other code relying on first expression
ROOT + 'any_all': 0,
}
),
(
......@@ -292,6 +308,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0, ## has other code relying on first expression
ROOT + 'any_all': 0,
}
),
(
......@@ -306,6 +323,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 1,
ROOT + 'any_all': 0,
}
),
(
......@@ -321,6 +339,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 1,
ROOT + 'any_all': 0,
}
),
(
......@@ -328,13 +347,13 @@ def test_misc():
if word is not None:
if len(word) > 20:
pass
if word is not None:
if len(word) > 20:
pass
else:
pass
if word is not None:
if len(word) > 20:
pass
......@@ -346,6 +365,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 1,
ROOT + 'any_all': 0,
}
),
(
......@@ -353,11 +373,11 @@ def test_misc():
if word is not None:
if len(word) > 20:
pass
if word is not None:
if len(word) > 20:
pass
if word is not None:
if len(word) > 20:
pass
......@@ -368,6 +388,7 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 3,
ROOT + 'any_all': 0,
}
),
(
......@@ -390,6 +411,93 @@ def test_misc():
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0,
}
),
(
dedent("""\
if apple or banana:
eat_fuit()
"""),
{
ROOT + 'if_else_overview': 1,
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0, ## not enough items
}
),
(
dedent("""\
if apple or banana or cherry:
eat_fuit()
"""),
{
ROOT + 'if_else_overview': 1,
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 1,
}
),
(
dedent("""\
if apple or banana or cherry and sausage:
eat_fuit()
"""),
{
ROOT + 'if_else_overview': 1,
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 1,
}
),
(
dedent("""\
if apple or cherry and sausage:
eat_fuit()
"""),
{
ROOT + 'if_else_overview': 1,
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 0, ## need at least one part to have 3 or more items
}
),
(
dedent("""\
for i in range(2):
if apple or banana or cherry and sausage:
eat_fuit()
"""),
{
ROOT + 'if_else_overview': 1,
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 1,
}
),
(
dedent("""\
for i in range(2):
if apple and banana and cherry and sausage:
eat_fuit()
"""),
{
ROOT + 'if_else_overview': 1,
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
ROOT + 'any_all': 1,
}
),
]
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment