Skip to content
This repository has been archived by the owner on Nov 10, 2018. It is now read-only.

docstring conform to PEP 484 #30

Merged
merged 2 commits into from
Aug 18, 2015
Merged

docstring conform to PEP 484 #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
06e3202
docstring conform to PEP 484
carocad Jul 1, 2015
d133460
merge conflict solve - Conform to PEP 484
carocad Aug 18, 2015
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
32 changes: 20 additions & 12 deletions scriptharness/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ class ScriptManager(object):
multiple globals that pylint would complain about.

Attributes:
all_scripts (dict): a dict of name:script
action_class (class): the class to instantiate for new actions
script_class (class): the class to instantiate for new scripts
all_scripts (Dict[str, scriptharness.script.Script]): a dict of name:script
action_class (scriptharness.actions.Action): the class to instantiate for new actions
script_class (scriptharness.script.Script): the class to instantiate for new scripts
"""
def __init__(self):
self.all_scripts = {}
Expand All @@ -59,6 +59,10 @@ def get_script(self, *args, **kwargs):

def get_config(self, name="root"):
"""Back end for scriptharness.get_config().

Args:
name (Optional[str]): The name of the script to get the config from.
Defaults to "root".
"""
if name not in self.all_scripts:
raise ScriptHarnessException(os.linesep.join([
Expand All @@ -74,6 +78,10 @@ def get_config(self, name="root"):

def get_logger(self, name="root"):
"""Back end for scriptharness.get_logger().

Args:
name (Optional[str]): The name of the script to get the config from.
Defaults to "root".
"""
if name not in self.all_scripts:
raise ScriptHarnessException(os.linesep.join([
Expand Down Expand Up @@ -104,8 +112,8 @@ def get_script(*args, **kwargs):
"""This will retrieve an existing script or create one and return it.

Args:
actions (tuple of Actions): When creating a new Script,
this is required. When retrieving an existing script, this is
actions (Tuple[scriptharness.actions.Action...]): When creating a new
Script, this is required. When retrieving an existing script, this is
ignored/optional.

parser (argparse.ArgumentParser): When creating a new Script,
Expand Down Expand Up @@ -136,7 +144,7 @@ def get_config(name="root"):
of name `name`.

Returns:
config (dict): By default scriptharness.structures.LoggingDict
config (Dict[str, str]): By default scriptharness.structures.LoggingDict
Copy link
Contributor

Choose a reason for hiding this comment

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

config doesn't have to have str values; they can be dicts, lists, strs, ints... maybe 'object' or 'various' or something?

Ah, looks like 'Any'.

"""
return MANAGER.get_config(name=name)

Expand All @@ -158,7 +166,7 @@ def get_logger(name="root"):
of name `name`.

Returns:
logger (logging.Logger)
logging.Logger: logger
"""
return MANAGER.get_logger(name=name)

Expand All @@ -185,11 +193,11 @@ def get_actions(all_actions):
"""Build a tuple of Action objects for the script.

Args:
all_actions (data structure): ordered mapping of action_name:enabled
all_actions (Sequence[str, bool]): ordered mapping of action_name:enabled
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be right. all_actions can be a tuple of tuples or list of lists or a dict. I'm not entirely sure Sequence is accurate, but I'm not sure how to improve it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I read in this case it would either be a Sequence or an Iterable, but to be honest the difference is also quite blurry for me. Do you think it would be better to have it as Iterable?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think leave it, for now. I think Sequence is better than Mapping, at least.

bool, as accepted by iterate_pairs()

Returns:
tuple of Action objects
Tuple[scriptharness.actions.Action, ...]: tuple of Action objects
"""
action_list = []
for action_name, value in iterate_pairs(all_actions):
Expand All @@ -201,11 +209,11 @@ def get_actions_from_list(all_actions, default_actions=None):
"""Helper method to generate the ordered mapping for get_actions().

Args:
all_actions (list): ordered list of all action names
default_actions (Optional[list]): actions that are enabled by default
all_actions (List[str]): ordered list of all action names
default_actions (Optional[List[str]]): actions that are enabled by default

Returns:
tuple of Action objects
Tuple[scriptharness.actions.Action, ...]: tuple of Action objects
"""
if default_actions is None:
default_actions = all_actions[:]
Expand Down
26 changes: 14 additions & 12 deletions scriptharness/actions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

Attributes:
LOGGER_NAME (str): logging.Logger name to use
STRINGS (dict): strings for actions. In the future these may be in a
function to allow for localization.
STRINGS (Dict[str, Dict[str, str]]): strings for actions. In the future
Copy link
Contributor

Choose a reason for hiding this comment

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

This is accurate at the moment, but really, it's just a freeform dict of dicts and strings. Are these docstrings really this inflexible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I preferred doing it this way because as you just said, it is a free form which is not clear for someone that hasn't seen the code at all. I think it is just more clear this way.

Nevertheless the docstrings are not inflexible at all, you could define a new type and use it there instead of the nested dicts. Another option is to define your own class and use that type (I did that for the LoggingDict) but since it was a free form I thought it was clearer this way.

these may be in a function to allow for localization.
"""
from __future__ import absolute_import, division, print_function, \
unicode_literals
Expand Down Expand Up @@ -43,7 +43,7 @@ def get_function_by_name(function_name):
function_name (str): The name of the function to find.

Returns:
function: the function found.
Callable[[Any], Any]: the function found.

Raises:
scriptharness.exceptions.ScriptHarnesException: if the function is
Expand All @@ -70,28 +70,29 @@ class Action(object):
enabled (bool): Enabled actions will run. Disabled actions will log
the skip_message and not run.

strings (dict): Strings for action-specific log messages.
strings (Dict[str, str]): Strings for action-specific log messages.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.


logger_name (str): The logger name for logging calls inside this object.

function (function): This is the function to call in run_function().
function (Callable[[Any], Any]): This is the function to call in
run_function().

history (dict): History of the action (return_value, status, start_time,
end_time).
history (Dict[str, Any]): History of the action (return_value, status,
start_time, end_time).
"""
def __init__(self, name, action_groups=None, function=None, enabled=True):
r"""Create the Action object.

Args:
name (str): Action name, for logging.

action_groups (list): a list of action group names that this
action_groups (List[str]): a list of action group names that this
Action belongs to. If scriptharness_volatile_action_group is
specified in config (usually via \--action-group), all actions
belonging to that action group will be enabled by default, and all
others disabled by default.

function (Optional[function]). This is the function or method
function (Optional[Callable[[Any], Any]]). This is the function or method
to run in run_function(). If not specified, use
get_function_by_name() to find the function that matches the
action name. If not found, raise.
Expand Down Expand Up @@ -124,8 +125,8 @@ def run_function(self, context):
This sets self.history['return_value'] for posterity.

Args:
context (Context): the context from the calling Script
(passed from run()).
context (Context): the context from the calling Script; as defined
in `script.py`. (passed from run()).
"""
self.history['return_value'] = self.function(context)

Expand All @@ -135,7 +136,8 @@ def run(self, context):
This sets self.history timestamps and status.

Args:
context (Context): the context from the calling Script.
context (Context): the context from the calling Script; as defined
in `script.py`

Returns:
status (int): one of SUCCESS, ERROR, or FATAL.
Expand Down
45 changes: 23 additions & 22 deletions scriptharness/commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

Attributes:
LOGGER_NAME (str): default logging.Logger name.
STRINGS (dict): Strings for logging.
STRINGS (Dict[str, Dict[str, str]]): Strings for logging.
"""
from __future__ import absolute_import, division, print_function, \
unicode_literals
Expand Down Expand Up @@ -75,8 +75,7 @@ def check_output(command, logger_name="scriptharness.commands.check_output",
"""Wrap subprocess.check_output with logging

Args:
command (str or list): The command to run.

command (str or List[str]): The command to run.
logger_name (Optional[str]): the logger name to log with.

level (Optional[int]): the logging level to log with. Defaults to
Expand Down Expand Up @@ -107,7 +106,7 @@ def detect_errors(command):
Otherwise it's unsuccessful.

Args:
command (Command obj):
command (Command): instance of `Command` class
"""
status = scriptharness.status.SUCCESS
return_value = command.history.get('return_value')
Expand All @@ -123,7 +122,7 @@ def detect_parsed_errors(command):
to 0, the command is successful. Otherwise it's unsuccessful.

Args:
command (Command obj):
command (Command): instance of `Command` class
"""
status = scriptharness.status.SUCCESS
if command.parser.history.get('num_errors'):
Expand All @@ -139,23 +138,23 @@ class Command(object):
scriptharness.commands.Output object.

Attributes:
command (list or string): The command to send to subprocess.Popen
command (List[str] or str): The command to send to subprocess.Popen

logger (logging.Logger): logger to log with.

detect_error_cb (function): this function determines whether the
command was successful.
detect_error_cb (Callable[[Any], scriptharness.status]): this function
determines whether the command was successful.

history (dict): This dictionary holds the timestamps and status of
history (Dict[str, Any]): This dictionary holds the timestamps and status of
the command.

kwargs (dict): These kwargs will be passed to subprocess.Popen, except
kwargs (Dict[Any, Any]): These kwargs will be passed to subprocess.Popen, except
for the optional 'output_timeout' and 'timeout', which are processed by
Command. `output_timeout` is how long a command can run without
outputting anything to the screen/log. `timeout` is how long the
command can run, total.

strings (dict): Strings to log.
strings (Dict[str, str]): Strings to log.
"""
def __init__(self, command, logger=None, detect_error_cb=None, **kwargs):
self.command = command
Expand All @@ -169,7 +168,7 @@ def log_env(self, env):
"""Log environment variables. Here for subclassing.

Args:
env (dict): the environment we'll be passing to subprocess.Popen.
env (Dict[str, str]): the environment we'll be passing to subprocess.Popen.
"""
env = self.fix_env(env)
self.logger.info(self.strings['env'], {'env': pprint.pformat(env)})
Expand All @@ -179,7 +178,10 @@ def fix_env(env):
"""Windows environments are fiddly.

Args:
env (dict): the environment we'll be passing to subprocess.Popen.
env (Dict[str, str]): the environment we'll be passing to subprocess.Popen.

Returns:
Dict[bytes, bytes]: copy of the env key and values but as byte strings
"""
if os.name == 'nt':
env.setdefault("SystemRoot", os.environ["SystemRoot"])
Expand Down Expand Up @@ -313,8 +315,7 @@ class Output(Command):
The output can be binary or text.

Attributes:
strings (dict): Strings to log.

strings (Dict[str, str]): Strings to log.
stdout (NamedTemporaryFile): file to log stdout to

stderr (NamedTemporaryFile): file to log stderr to
Expand Down Expand Up @@ -418,7 +419,7 @@ def run(command, cmd_class=Command, halt_on_failure=False, *args, **kwargs):
are explicitly trying to kill the script.

Args:
command (list or str): Command line to run.
command (List[str] or str): Command line to run.

cmd_class (Optional[Command subclass]): the class to instantiate.
Defaults to scriptharness.commands.Command.
Expand All @@ -429,7 +430,7 @@ def run(command, cmd_class=Command, halt_on_failure=False, *args, **kwargs):
**kwargs: kwargs for subprocess.Popen.

Returns:
command exit code (int)
int: command exit code

Raises:
scriptharness.exceptions.ScriptHarnessFatal: on fatal error
Expand Down Expand Up @@ -460,12 +461,12 @@ def parse(command, **kwargs):
are explicitly trying to kill the script.

Args:
command (list or str): Command line to run.
command (List[str] or str): Command line to run.

**kwargs: kwargs for run/ParsedCommand.

Returns:
command exit code (int)
int: command exit code

Raises:
scriptharness.exceptions.ScriptHarnessFatal: on fatal error
Expand All @@ -488,7 +489,7 @@ def get_output(command, halt_on_failure=False, **kwargs):
scriptharness.unicode.to_unicode().

Args:
command (list or str): the command to use in subprocess.Popen
command (List[str] or str): the command to use in subprocess.Popen

halt_on_failure (Optional[bool]): raise ScriptHarnessFatal on error
if True. Default: False
Expand Down Expand Up @@ -528,14 +529,14 @@ def get_text_output(command, level=logging.INFO, **kwargs):
Because we log the output, we're assuming the output is text.

Args:
command (list or str): command for subprocess.Popen
command (List[str] or str): command for subprocess.Popen

level (int): logging level

**kwargs: kwargs to send to scriptharness.commands.Output

Returns:
output (text): the stdout from the command.
output (str): the stdout from the command.
"""
cmd = Output(command, **kwargs)
with get_output(command, **kwargs) as cmd:
Expand Down