Skip to content

Commit

Permalink
YAPF, take 3 (ray-project#2098)
Browse files Browse the repository at this point in the history
* Use pep8 style

The original style file is actually just pep8 style, but with everything
spelled out. It's easier to use the `based_on_style` feature. Any overrides are
clearer that way.

* Improve yapf script

1. Do formatting in parallel
2. Lint RLlib
3. Use .style.yapf file

* Pull out expressions into variables

* Don't format rllib

* Don't allow splits in dicts

* Apply yapf

* Disallow single line if-statements

* Use arithmetic comparison

* Simplify checking for changed files

* Pull out expr into var
  • Loading branch information
alok authored and richardliaw committed May 19, 2018
1 parent 8e0962b commit 9a8f29e
Show file tree
Hide file tree
Showing 14 changed files with 218 additions and 500 deletions.
191 changes: 3 additions & 188 deletions .style.yapf
Original file line number Diff line number Diff line change
@@ -1,189 +1,4 @@
[style]
# Align closing bracket with visual indentation.
align_closing_bracket_with_visual_indent=True

# Allow dictionary keys to exist on multiple lines. For example:
#
# x = {
# ('this is the first element of a tuple',
# 'this is the second element of a tuple'):
# value,
# }
allow_multiline_dictionary_keys=False

# Allow lambdas to be formatted on more than one line.
allow_multiline_lambdas=False

# Insert a blank line before a class-level docstring.
blank_line_before_class_docstring=False

# Insert a blank line before a 'def' or 'class' immediately nested
# within another 'def' or 'class'. For example:
#
# class Foo:
# # <------ this blank line
# def method():
# ...
blank_line_before_nested_class_or_def=False

# Do not split consecutive brackets. Only relevant when
# dedent_closing_brackets is set. For example:
#
# call_func_that_takes_a_dict(
# {
# 'key1': 'value1',
# 'key2': 'value2',
# }
# )
#
# would reformat to:
#
# call_func_that_takes_a_dict({
# 'key1': 'value1',
# 'key2': 'value2',
# })
coalesce_brackets=True

# The column limit.
column_limit=79

# Indent width used for line continuations.
continuation_indent_width=4

# Put closing brackets on a separate line, dedented, if the bracketed
# expression can't fit in a single line. Applies to all kinds of brackets,
# including function definitions and calls. For example:
#
# config = {
# 'key1': 'value1',
# 'key2': 'value2',
# } # <--- this bracket is dedented and on a separate line
#
# time_series = self.remote_client.query_entity_counters(
# entity='dev3246.region1',
# key='dns.query_latency_tcp',
# transform=Transformation.AVERAGE(window=timedelta(seconds=60)),
# start_ts=now()-timedelta(days=3),
# end_ts=now(),
# ) # <--- this bracket is dedented and on a separate line
dedent_closing_brackets=False

# Place each dictionary entry onto its own line.
each_dict_entry_on_separate_line=True

# The regex for an i18n comment. The presence of this comment stops
# reformatting of that line, because the comments are required to be
# next to the string they translate.
i18n_comment=

# The i18n function call names. The presence of this function stops
# reformattting on that line, because the string it has cannot be moved
# away from the i18n comment.
i18n_function_call=

# Indent the dictionary value if it cannot fit on the same line as the
# dictionary key. For example:
#
# config = {
# 'key1':
# 'value1',
# 'key2': value1 +
# value2,
# }
indent_dictionary_value=True

# The number of columns to use for indentation.
indent_width=4

# Join short lines into one line. E.g., single line 'if' statements.
join_multiple_lines=True

# Do not include spaces around selected binary operators. For example:
#
# 1 + 2 * 3 - 4 / 5
#
# will be formatted as follows when configured with a value "*,/":
#
# 1 + 2*3 - 4/5
#
no_spaces_around_selected_binary_operators=set([])

# Use spaces around default or named assigns.
spaces_around_default_or_named_assign=False

# Use spaces around the power operator.
spaces_around_power_operator=False

# The number of spaces required before a trailing comment.
spaces_before_comment=2

# Insert a space between the ending comma and closing bracket of a list,
# etc.
space_between_ending_comma_and_closing_bracket=True

# Split before arguments if the argument list is terminated by a
# comma.
split_arguments_when_comma_terminated=False

# Set to True to prefer splitting before '&', '|' or '^' rather than
# after.
split_before_bitwise_operator=True

# Split before a dictionary or set generator (comp_for). For example, note
# the split before the 'for':
#
# foo = {
# variable: 'Hello world, have a nice day!'
# for variable in bar if variable != 42
# }
split_before_dict_set_generator=True

# If an argument / parameter list is going to be split, then split before
# the first argument.
split_before_first_argument=False

# Set to True to prefer splitting before 'and' or 'or' rather than
# after.
split_before_logical_operator=True

# Split named assignments onto individual lines.
split_before_named_assigns=True

# The penalty for splitting right after the opening bracket.
split_penalty_after_opening_bracket=30

# The penalty for splitting the line after a unary operator.
split_penalty_after_unary_operator=10000

# The penalty for splitting right before an if expression.
split_penalty_before_if_expr=0

# The penalty of splitting the line around the '&', '|', and '^'
# operators.
split_penalty_bitwise_operator=300

# The penalty for characters over the column limit.
split_penalty_excess_character=4500

# The penalty incurred by adding a line split to the unwrapped line. The
# more line splits added the higher the penalty.
split_penalty_for_added_line_split=30

# The penalty of splitting a list of "import as" names. For example:
#
# from a_very_long_or_indented_module_name_yada_yad import (long_argument_1,
# long_argument_2,
# long_argument_3)
#
# would reformat to something like:
#
# from a_very_long_or_indented_module_name_yada_yad import (
# long_argument_1, long_argument_2, long_argument_3)
split_penalty_import_names=0

# The penalty of splitting the line around the 'and' and 'or'
# operators.
split_penalty_logical_operator=300

# Use the Tab character for indentation.
use_tabs=False
based_on_style=pep8
allow_split_before_dict_value=False
join_multiple_lines=False
40 changes: 21 additions & 19 deletions .travis/yapf.sh
Original file line number Diff line number Diff line change
@@ -1,27 +1,29 @@
#!/usr/bin/env bash

# Cause the script to exit if a single command fails
set -e
set -eo pipefail

ROOT_DIR=$(cd "$(dirname "${BASH_SOURCE:-$0}")"; pwd)
# this stops git rev-parse from failing if we run this from the .git directory
builtin cd "$(dirname "${BASH_SOURCE:-$0}")"

pushd $ROOT_DIR/../test
find . -name '*.py' -type f -exec yapf --style=pep8 -i -r {} \;
popd
ROOT="$(git rev-parse --show-toplevel)"
builtin cd "$ROOT"

pushd $ROOT_DIR/../python
find . -name '*.py' -type f -not -path './ray/dataframe/*' -not -path './ray/rllib/*' -not -path './ray/cloudpickle/*' -exec yapf --style=pep8 -i -r {} \;
popd
yapf \
--style "$ROOT/.style.yapf" \
--in-place --recursive --parallel \
--exclude 'python/ray/cloudpickle' \
--exclude 'python/ray/dataframe' \
--exclude 'python/ray/rllib' \
-- \
'test' 'python'

CHANGED_FILES=(`git diff --name-only`)
if [ "$CHANGED_FILES" ]; then
echo 'Reformatted staged files. Please review and stage the changes.'
echo
echo 'Files updated:'
for file in ${CHANGED_FILES[@]}; do
echo " $file"
done
exit 1
else
exit 0
if ! git diff --quiet; then
echo 'Reformatted staged files. Please review and stage the changes.'
echo 'Files updated:'
echo

git --no-pager diff --name-only

exit 1
fi
40 changes: 14 additions & 26 deletions python/ray/actor.py
Original file line number Diff line number Diff line change
Expand Up @@ -857,32 +857,20 @@ def _serialization_helper(self, ray_forking):
A dictionary of the information needed to reconstruct the object.
"""
state = {
"actor_id":
self._ray_actor_id.id(),
"class_name":
self._ray_class_name,
"actor_forks":
self._ray_actor_forks,
"actor_cursor":
self._ray_actor_cursor.id(),
"actor_counter":
0, # Reset the actor counter.
"actor_method_names":
self._ray_actor_method_names,
"method_signatures":
self._ray_method_signatures,
"method_num_return_vals":
self._ray_method_num_return_vals,
"actor_creation_dummy_object_id":
self._ray_actor_creation_dummy_object_id.id(),
"actor_method_cpus":
self._ray_actor_method_cpus,
"actor_driver_id":
self._ray_actor_driver_id.id(),
"previous_actor_handle_id":
self._ray_actor_handle_id.id(),
"ray_forking":
ray_forking
"actor_id": self._ray_actor_id.id(),
"class_name": self._ray_class_name,
"actor_forks": self._ray_actor_forks,
"actor_cursor": self._ray_actor_cursor.id(),
"actor_counter": 0, # Reset the actor counter.
"actor_method_names": self._ray_actor_method_names,
"method_signatures": self._ray_method_signatures,
"method_num_return_vals": self._ray_method_num_return_vals,
"actor_creation_dummy_object_id": self.
_ray_actor_creation_dummy_object_id.id(),
"actor_method_cpus": self._ray_actor_method_cpus,
"actor_driver_id": self._ray_actor_driver_id.id(),
"previous_actor_handle_id": self._ray_actor_handle_id.id(),
"ray_forking": ray_forking
}

if ray_forking:
Expand Down
12 changes: 4 additions & 8 deletions python/ray/autoscaler/autoscaler.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,19 +182,15 @@ def _info(self):
nodes_used += max_frac
idle_times = [now - t for t in self.last_used_time_by_ip.values()]
return {
"ResourceUsage":
", ".join([
"ResourceUsage": ", ".join([
"{}/{} {}".format(
round(resources_used[rid], 2),
round(resources_total[rid], 2), rid)
for rid in sorted(resources_used)
]),
"NumNodesConnected":
len(self.static_resources_by_ip),
"NumNodesUsed":
round(nodes_used, 2),
"NodeIdleSeconds":
"Min={} Mean={} Max={}".format(
"NumNodesConnected": len(self.static_resources_by_ip),
"NumNodesUsed": round(nodes_used, 2),
"NodeIdleSeconds": "Min={} Mean={} Max={}".format(
int(np.min(idle_times)) if idle_times else -1,
int(np.mean(idle_times)) if idle_times else -1,
int(np.max(idle_times)) if idle_times else -1),
Expand Down
31 changes: 15 additions & 16 deletions python/ray/autoscaler/aws/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,22 +209,21 @@ def _configure_security_group(config):
assert security_group, "Failed to create security group"

if not security_group.ip_permissions:
security_group.authorize_ingress(
IpPermissions=[{
"FromPort": -1,
"ToPort": -1,
"IpProtocol": "-1",
"UserIdGroupPairs": [{
"GroupId": security_group.id
}]
}, {
"FromPort": 22,
"ToPort": 22,
"IpProtocol": "TCP",
"IpRanges": [{
"CidrIp": "0.0.0.0/0"
}]
}])
security_group.authorize_ingress(IpPermissions=[{
"FromPort": -1,
"ToPort": -1,
"IpProtocol": "-1",
"UserIdGroupPairs": [{
"GroupId": security_group.id
}]
}, {
"FromPort": 22,
"ToPort": 22,
"IpProtocol": "TCP",
"IpRanges": [{
"CidrIp": "0.0.0.0/0"
}]
}])

if "SecurityGroupIds" not in config["head_node"]:
print("SecurityGroupIds not specified for head node, using {}".format(
Expand Down
9 changes: 3 additions & 6 deletions python/ray/autoscaler/aws/node_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,9 @@ def create_node(self, node_config, tags, count):
"Value": v,
})
conf.update({
"MinCount":
1,
"MaxCount":
count,
"TagSpecifications":
conf.get("TagSpecifications", []) + [{
"MinCount": 1,
"MaxCount": count,
"TagSpecifications": conf.get("TagSpecifications", []) + [{
"ResourceType": "instance",
"Tags": tag_pairs,
}]
Expand Down
6 changes: 2 additions & 4 deletions python/ray/autoscaler/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,8 @@ def get_or_create_head_node(config, no_restart, yes):
remote_config_file.write(json.dumps(remote_config))
remote_config_file.flush()
config["file_mounts"].update({
remote_key_path:
config["auth"]["ssh_private_key"],
"~/ray_bootstrap_config.yaml":
remote_config_file.name
remote_key_path: config["auth"]["ssh_private_key"],
"~/ray_bootstrap_config.yaml": remote_config_file.name
})

if no_restart:
Expand Down
Loading

0 comments on commit 9a8f29e

Please sign in to comment.