Commit 361fb59d authored by Grant Paton-Simpson's avatar Grant Paton-Simpson

Add context manager advisors; misc

* Add context manager advisors
* Add comment on aspect-oriented programming for both decorators and context managers
* Shift code from shared to advisors.__init__
* Shorten repeated_message to repeat
* Add space before H1 in terminal output
* Bump version
parent 21f0e36a
Pipeline #565 failed with stages
......@@ -13,4 +13,4 @@ upload:
rm -f dist/*
/home/g/projects/superhelp/superhelp/env/bin/python3 setup.py sdist bdist_wheel
/home/g/projects/superhelp/superhelp/env/bin/python3 -m twine upload dist/* example_*.png
/home/g/projects/superhelp/superhelp/env/bin/python3 -m twine upload dist/*
# https://git.nzoss.org.nz/pyGrant/superhelp
version number: 0.9.4
version number: 0.9.5
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.4'
__version__ = '0.9.5'
here = path.abspath(path.dirname(__file__))
......
......@@ -36,13 +36,17 @@ E.g. dedent(
Subtle huh!!? And you thought whitespace in Python was a risk!
"""
import builtins
from collections import namedtuple
from importlib import import_module
import keyword
from pkgutil import iter_modules
import sys
from textwrap import dedent
from .. import conf
from ..utils import layout_comment as layout
FILT_BLOCK_ADVISORS = [] ## block-based advisors which only apply to blocks filtered to contain specified element types
......@@ -135,3 +139,278 @@ def load_advisors():
this_module.__name__ + '.')
for submodule in submodules:
import_module(submodule.name)
## =============================================================================
def is_reserved_name(name):
is_reserved = name in set(keyword.kwlist + dir(builtins) + conf.STD_LIBS)
return is_reserved
AOP_COMMENT = layout("""
Some aspects of code apply to lots of different parts of the code base e.g.
logging, or security. These can be thought of as "cross-cutting concerns"
from an Aspect-Oriented Programming point of view. The problem is that a lot
of code gets repeated e.g. if every function has to implement its own
logging or has to handle its own security. Even if there is some shared
functionality it can often require special wiring into other code. In
short, coping with cross-cutting concerns can be painful.
Decorators and Context Managers are ways Python provides of concentrating
cross-cutting concerns into single pieces of code in an elegant way. They
DRY out your code (see
<https://en.wikipedia.org/wiki/Don%27t_repeat_yourself>). And because they
are very widely used, it is worth learning how to use them, even if you
don't take the step of writing your own.
""")
UNPACKING_COMMENT = (
layout("""\
Unpacking is much more pythonic than using indexes to pull a sequence
apart into names (variables). For example:
""")
+
layout("""\
#### Un-pythonic :-(
location = (-37, 174, 'Auckland', 'Mt Albert')
lat = location[0]
lon = location[1]
city = location[2]
suburb = location[3]
#### Pythonic :-)
lat, lon, city, suburb = location
""", is_code=True)
+
layout(f"""\
If you don't need all the values you can indicate which you want to
ignore or even mop up multiple unused values into a single value using
an asterisk.
For example:
""")
+
layout("""\
lat, lon, _city, _suburb = location
""", is_code=True)
+
layout(f"""\
or:
""")
+
layout(f"""\
lat, lon, *_ = location
""", is_code=True)
+
layout("""\
or:
""")
+
layout("""\
lat, lon, *unused = location
""", is_code=True)
+
layout(f"""\
Note - unused, in this case, would be ['Auckland', 'Mt Albert']
""")
)
GENERAL_COMPREHENSION_COMMENT = layout("""\
Comprehensions are one the great things about Python. To see why, have a
look at Raymond Hettinger's classic talk "Transforming Code into Beautiful,
Idiomatic Python" <https://youtu.be/OSGv2VnC0go?t=2738> where he explains
the rationale. In short, if the goal of your code can be expressed as a
single English sentence then it might belong on one line. The code should
say what it is doing more than how it is doing it. Comprehensions are
declarative and that is A Very Good Thing™.
Pro tip: don't make comprehensions *in*comprehensions ;-). If your
comprehension is hard to read it is probably better rewritten as a looping
structure.
""")
LIST_COMPREHENSION_COMMENT = (
layout("""\
#### Example List Comprehension:
""")
+
layout("""\
names_lengths = [
len(name)
for name in ['Tinky Winky', 'Dipsy', 'La La', 'Po']
]
""", is_code=True)
+
layout("""\
produces an ordinary list:
""")
+
layout(str(
{
len(name)
for name in ['Tinky Winky', 'Dipsy', 'La La', 'Po']
}
))
+
layout("""\
It is also possible to add a simple filter using `if`
""")
+
layout("""\
names_lengths = [
len(name)
for name in ['Tinky Winky', 'Dipsy', 'La La', 'Po']
if not name.startswith('T')
]
""", is_code=True)
+
layout("""\
produces an ordinary list:
""")
+
layout(str(
{
len(name)
for name in ['Tinky Winky', 'Dipsy', 'La La', 'Po']
if not name.startswith('T')
}
))
)
DICT_COMPREHENSION_COMMENT = (
layout("""\
#### Example Dictionary Comprehension:
""")
+
layout("""\
country2capital = {{
country: capital
for country, capital in [('NZ', 'Wellington'), ('Italy', 'Rome')]
}}
""", is_code=True)
+
layout("""\
produces an ordinary dictionary:
""")
+
layout(str(
{
country: capital
for country, capital
in [('NZ', 'Wellington'), ('Italy', 'Rome')]
}
))
+
layout("""\
It is also possible to add a simple filter using `if`
""")
+
layout("""\
country2capital = {{
country: capital
for country, capital in [('NZ', 'Wellington'), ('Italy', 'Rome')]
if country == 'NZ'
}}
""", is_code=True)
+
layout("""\
produces an ordinary dictionary:
""")
+
layout(str(
{
country: capital
for country, capital
in [('NZ', 'Wellington'), ('Italy', 'Rome')]
if country == 'NZ'
}
))
)
SET_COMPREHENSION_COMMENT = (
layout("""\
#### Example Set Comprehension
""")
+
layout("""\
pets = {{
pet for _person, pet
in [('Rachel', 'cat'), ('Elliot', 'goat'), ('Giles', 'cat'),]
}}
""", is_code=True)
+
layout("""\
produces an ordinary set (i.e. unique members only):
""")
+
layout(str(
{
pet for _person, pet
in [('Rachel', 'cat'), ('Elliot', 'goat'), ('Giles', 'cat'),]
}
))
+
layout("""\
It is also possible to add a simple filter using `if`
""")
+
layout("""\
pets = {{
pet for person, pet
in [('Rachel', 'cat'), ('Elliot', 'goat'), ('Giles', 'cat'),]
if person != 'Elliot'
}}
""", is_code=True)
+
layout("""\
produces an ordinary set (i.e. unique members only):
""")
+
layout(str(
{
pet for person, pet
in [('Rachel', 'cat'), ('Elliot', 'goat'), ('Giles', 'cat'),]
if person != 'Elliot'
}
))
)
......@@ -10,7 +10,7 @@ from ..utils import get_nice_str_list, layout_comment as layout
CLASS_XPATH = ('descendant-or-self::ClassDef')
@filt_block_advisor(xpath=CLASS_XPATH, warning=True)
def getters_setters(block_dets, *, repeated_message=False):
def getters_setters(block_dets, *, repeat=False):
"""
Look for getters and setters and suggest @property if appropriate.
"""
......@@ -52,7 +52,7 @@ def getters_setters(block_dets, *, repeated_message=False):
looks like a {method_type}.
"""))
simple_class_msg = ''.join(simple_class_msg_bits)
if not repeated_message:
if not repeat:
properties_option = layout("""\
Python doesn't need getters and setters. Instead, you can use
......@@ -201,7 +201,7 @@ def getters_setters(block_dets, *, repeated_message=False):
return message
@filt_block_advisor(xpath=CLASS_XPATH, warning=True)
def selfless_methods(block_dets, *, repeated_message=False):
def selfless_methods(block_dets, *, repeat=False):
"""
Look for class methods that don't use self as candidates for @staticmethod
decorator. Note - not worried about detecting sophisticated cases with
......@@ -250,7 +250,7 @@ def selfless_methods(block_dets, *, repeated_message=False):
doesn't use the instance (usually called `self`).
"""))
simple_class_msg = ''.join(simple_class_msg_bits)
if not repeated_message:
if not repeat:
staticmethod_msg = layout("""\
If a method doesn't use the instance it can be either pulled into a
......@@ -304,7 +304,7 @@ def selfless_methods(block_dets, *, repeated_message=False):
return message
@filt_block_advisor(xpath=CLASS_XPATH, warning=True)
def one_method_classes(block_dets, *, repeated_message=False):
def one_method_classes(block_dets, *, repeat=False):
"""
Look for classes with only one method (other than __init__) and suggest a
simple function as an alternative..
......@@ -349,7 +349,7 @@ def one_method_classes(block_dets, *, repeated_message=False):
- {class_name}: {method2use}
"""))
n_methods_msg = ''.join(n_methods_msg_bits)
if not repeated_message:
if not repeat:
not_just_oo = layout("""\
Python allows procedural, object-oriented, and functional styles of
programming. Event-based programming is also used in GUI contexts,
......
from ..advisors import filt_block_advisor
from .. import conf
from ..advisors import AOP_COMMENT
from ..utils import layout_comment as layout
LONG_EXAMPLE_OPEN_CM = (
layout("""\
For example, every time we open a file we need to make sure it is closed
when we are finished. It is not that hard to handle that yourself in
simple cases but can become much harder - perhaps the code exits in
multiple places (perhaps as a result of an `exception`). You have to
remember to handle the tidy-up in every place an exit might occur.
Either you will forget one of them or you'll be repeating code all over
the place violating the DRY principle - namely:
> "Don't Repeat Yourself" (The Pragmatic Programmer)
WET presumably stands for "Write Every Time", "Write Everything Twice",
"We Enjoy Typing" or "Waste Everyone's Time" ;-)
If you don't use a context manager, problems can emerge as a snippet
gradually evolves. And it usually all starts so innocently :-). Here's
one example of the problem unfolding:
""")
+
layout("""\
## 1) Not using context manager (with) not really a problem
f = open(fname)
text = f.read()
f.close() # <------------------ First one (I hope this is the only one)
## 2) Maybe we need to handle empty files
f = open(fname)
text = f.read()
if not text:
raise Exception(f"{fname} is empty") # <------ errr ... is the file
f.close() # handle going to get closed?
## 3) We remember to close the file handle if an exception
f = open(fname)
text = f.read()
if not text:
f.close() # <---------------------------- Rats! I need a second one
raise Exception(f"{fname} is empty")
f.close()
## 4) We want the index of TAG for some reason
f = open(fname)
text = f.read()
if not text:
f.close()
raise Exception(f"{fname} is empty")
idx = text.index(TAG) # <--------------- Hmmm - will raise ValueError if
f.close() # TAG not in text? File left open?
## 5) We remember to handle the exception
f = open(fname)
text = f.read()
if not text:
f.close()
raise Exception(f"{fname} is empty")
try:
idx = text.index(TAG)
except ValueError:
f.close() # <----------------- Three already! I'm lucky it's only a
raise # one-liner each time.
f.close()
""", is_code=True)
+
layout("""\
The code is much cleaner and robust using a simple context manager :-)
""")
+
layout("""\
with open(fname) as f:
text = f.read()
if not text:
raise Exception(f"{fname} is empty")
idx = text.index(TAG)
## If my code gets here, i.e. past the indented block inside the context
## manager, we are guaranteed to have freed up the file - nice!
""", is_code=True)
)
WITH_XPATH = 'descendant-or-self::With'
def is_using_open(with_el):
func_name_els = with_el.xpath(
'descendant::items/withitem/context_expr/Call/func/Name')
if len(func_name_els) != 1:
return False
func_name_el = func_name_els[0]
func_name = func_name_el.get('id')
return func_name == 'open'
@filt_block_advisor(xpath=WITH_XPATH)
def content_manager_overview(block_dets, *, repeat=False):
"""
Explain context managers.
"""
with_els = block_dets.element.xpath(WITH_XPATH)
using_open_cm = any([is_using_open(with_el) for with_el in with_els])
if not with_els:
return None
title = layout("""\
### Context manager(s) used
""")
if using_open_cm:
summary = layout("""\
Your code includes the commonly used file opening context manager.
""")
else:
summary = layout("""\
Your code uses a context manager.
""")
brief_usage = layout("""\
Context managers take care of anything required when we enter a
block of code or exit it.
""")
if not repeat:
brief_example = layout("""\
For example, every time we open a file we need to make sure it is
closed when we are finished. Or when we finish with database
connections and cursors we need to clean up after ourselves. There
may be some things we need to do when we start a code block as well.
""")
long_example = LONG_EXAMPLE_OPEN_CM
aop = AOP_COMMENT
else:
brief_example = ''
long_example = ''
aop = ''
message = {
conf.BRIEF: title + summary + brief_usage + brief_example,
conf.MAIN: title + summary + brief_usage + long_example,
conf.EXTRA: aop,
}
return message
ASSIGN_OPEN_XPATH = 'descendant-or-self::Assign/value/Call/func/Name'
@filt_block_advisor(xpath=ASSIGN_OPEN_XPATH, warning=True)
def file_cm_needed(block_dets, *, repeat=False):
"""
Look for opening of file without a context managers - recommend use of the
"with open" context manager.
"""
assign_open_els = block_dets.element.xpath(ASSIGN_OPEN_XPATH)
if not assign_open_els:
return None
title = layout("""\
### File opened without context manager
""")
summary = layout("""
Your code opens a file without using a context manager.
""")
if not repeat:
reasons = layout("""\
Using a context manager is easy (actually, writing them isn't even
that hard) but using the standard ones has big advantages.
""")
long_example = LONG_EXAMPLE_OPEN_CM
aop = AOP_COMMENT
else:
reasons = ''
long_example = ''
aop = ''
message = {
conf.BRIEF: title + summary,
conf.MAIN: title + summary + reasons + long_example,
conf.EXTRA: aop,
}
return message
from ..advisors import filt_block_advisor
from .. import conf
from ..advisors import AOP_COMMENT
from ..utils import get_nice_str_list, layout_comment as layout
DECORATOR_XPATH = (
......@@ -11,7 +12,7 @@ DECORATOR_XPATH = (
)
@filt_block_advisor(xpath=DECORATOR_XPATH)
def decorator_overview(block_dets, *, repeated_message=False):
def decorator_overview(block_dets, *, repeat=False):
"""
Look for decorators and explain some options for improving them.
"""
......@@ -36,7 +37,7 @@ def decorator_overview(block_dets, *, repeated_message=False):
The code uses the decorator{plural}: {dec_name_list}.
""")
if not repeated_message:
if not repeat:
dec_dets = (
layout("""\
......@@ -95,11 +96,14 @@ def decorator_overview(block_dets, *, repeated_message=False):
say("sausage!")
''', is_code=True)
)
aop = AOP_COMMENT
else:
dec_dets = ''
aop = ''
message = {
conf.BRIEF: summary,
conf.MAIN: summary + dec_dets,
conf.EXTRA: aop,
}
return message
......@@ -6,7 +6,7 @@ from ..utils import get_nice_str_list, layout_comment as layout
ASSIGN_DICT_XPATH = 'descendant-or-self::Assign/value/Dict'
@filt_block_advisor(xpath=ASSIGN_DICT_XPATH)
def dict_overview(block_dets, *, repeated_message=False):
def dict_overview(block_dets, *, repeat=False):
"""
Look at assigned dictionaries e.g. location = {'country' 'New Zealand',
'city': 'Auckland'}
......@@ -29,7 +29,7 @@ def dict_overview(block_dets, *, repeated_message=False):
(i.e. {utils.int2nice(len(items))} mappings).
"""))
brief_desc = ''.join(brief_desc_bits)
if not repeated_message:
if not repeat:
dict_def = layout("""\
Dictionaries map keys to values.
......@@ -151,7 +151,7 @@ def get_key_type_names(items):
return key_type_names, key_type_nice_names
@filt_block_advisor(xpath=ASSIGN_DICT_XPATH, warning=True)
def mixed_key_types(block_dets, *, repeated_message=False):
def mixed_key_types(block_dets, *, repeat=False):
"""
Warns about dictionaries with mix of string and integer keys.
"""
......@@ -189,7 +189,7 @@ def mixed_key_types(block_dets, *, repeated_message=False):
`{name}`'s keys include both strings and integers which is probably
a bad idea.
""")
if not repeated_message:
if not repeat:
one_vs_1 = layout("""\
For example, if you have both 1 and "1" as keys in a dictionary
......