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.

Sign up for GitHub

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

Jump to bottom

Format all code using black #534

Closed
wants to merge 1 commit into from
Closed

Format all code using black #534

wants to merge 1 commit into from

Conversation

lihuoran
Copy link
Contributor

Description

Linked issue(s)/Pull request(s)

Type of Change

  • Non-breaking bug fix
  • Breaking bug fix
  • New feature
  • Test
  • Doc update
  • Docker update

Related Component

  • Simulation toolkit
  • RL toolkit
  • Distributed toolkit

Has Been Tested

  • OS:
    • Windows
    • Mac OS
    • Linux
  • Python version:
    • 3.6
    • 3.7
  • Key information snapshot(s):

Needs Follow Up Actions

  • New release package
  • New docker image

Checklist

  • Add/update the related comments
  • Add/update the related tests
  • Add/update the related documentations
  • Update the dependent downstream modules usage

@lihuoran lihuoran requested a review from Jinyu-W May 26, 2022 03:51
lihuoran
lihuoran commented May 26, 2022
View reviewed changes
maro/cli/k8s/executors/k8s_aks_executor.py
"name": "PYTHONUNBUFFERED",
"value": "0"
}
{"name": "CLUSTER_ID", "value": f"{self.cluster_id}"},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

mark

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#fmt: off
#fmt: on

@Jinyu-W Jinyu-W requested review from v-heli and VictorYXL May 30, 2022 02:46
Jinyu-W
Jinyu-W reviewed May 30, 2022
View reviewed changes
examples/cim/rl/config.py
"topology": "toy.4p_ssdd_l0.0",
"durations": 560
}
env_conf = {"scenario": "cim", "topology": "toy.4p_ssdd_l0.0", "durations": 560}
Copy link
Collaborator

Choose a reason for hiding this comment

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

mark

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ignore this file in the command.

examples/cim/rl/config.py
(state_shaping_conf["look_back"] + 1) * (state_shaping_conf["max_ports_downstream"] + 1) * len(port_attributes)
+ len(vessel_attributes)
)
state_dim = (state_shaping_conf["look_back"] + 1) * (state_shaping_conf["max_ports_downstream"] + 1) * len(
Copy link
Collaborator

Choose a reason for hiding this comment

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

mark

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add related descriptions into wiki.

Jinyu-W
Jinyu-W reviewed May 30, 2022
View reviewed changes
examples/vector_env/hello.py
@@ -28,7 +30,7 @@ class VectorEnvUsage(Enum):

# Showcase: how to access information from snapshot list in vector env.
if env0_dec:
ss0 = env.snapshot_list["vessels"][env0_dec.tick:env0_dec.vessel_idx:"remaining_space"]
ss0 = env.snapshot_list["vessels"][env0_dec.tick : env0_dec.vessel_idx : "remaining_space"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mark.

Compared with line 83 in examples/simulator/data_model.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See this doc for explanations. I personally buy it.

Jinyu-W
Jinyu-W reviewed May 30, 2022
View reviewed changes
maro/cli/grass/lib/services/node_agent/agent.py
@@ -382,30 +368,29 @@ def load_images(self) -> None:
for image_file_name, image_file_details in name_to_image_file_details_in_master.items():
if (
image_file_name not in name_to_image_file_details_in_node
or name_to_image_file_details_in_node[image_file_name]["md5_checksum"] !=
image_file_details["md5_checksum"]
or name_to_image_file_details_in_node[image_file_name]["md5_checksum"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

anyway to add () in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shorten variable name.

Jinyu-W
Jinyu-W reviewed May 30, 2022
View reviewed changes
maro/cli/grass/lib/services/node_agent/agent.py

with self._redis_controller.lock(f"lock:name_to_node_details:{self._local_node_details['name']}"):
node_details = self._redis_controller.get_node_details(node_name=self._local_node_details["name"])
# Update with mapping in master.
node_details["image_files"] = name_to_image_file_details_in_master
self._redis_controller.set_node_details(
node_name=self._local_node_details["name"],
node_details=node_details
node_name=self._local_node_details["name"], node_details=node_details
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add tailing , or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See this

Jinyu-W
Jinyu-W reviewed May 30, 2022
View reviewed changes
maro/rl/rollout/env_sampler.py
next_agent_state_dict={
agent_name: self.next_agent_state_dict[agent_name]
} if self.next_agent_state_dict is not None and agent_name in self.next_agent_state_dict else {},
next_agent_state_dict={agent_name: self.next_agent_state_dict[agent_name]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

mark

Copy link
Contributor Author

Choose a reason for hiding this comment

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

[TBD] Do not DINGGE.

Jinyu-W
Jinyu-W reviewed May 30, 2022
View reviewed changes
maro/simulator/scenarios/cim/common.py
PARKING = 0
SAILING = 1


class ActionType(Enum):
"""Type of CIM action."""
LOAD = "load",

LOAD = ("load",)
Copy link
Collaborator

Choose a reason for hiding this comment

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

mark

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix this.

Jinyu-W
Jinyu-W reviewed May 30, 2022
View reviewed changes
maro/utils/exception/error_code.py
@@ -19,11 +19,9 @@
1011: "Peers Disconnection Error",
1012: "MARO Send Again Error",
1013: "Peers Rejoin Timeout",

Copy link
Collaborator

Choose a reason for hiding this comment

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

mark

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exclude this file.

Jinyu-W
Jinyu-W reviewed May 30, 2022
View reviewed changes
maro/vector_env/env_process.py
@@ -26,7 +26,7 @@ def __init__(self, pipe: Connection, *args, **kwargs):
def run(self):
"""Initialize environment and process commands."""
metrics = None
decision_event = None,
decision_event = (None,)
Copy link
Collaborator

Choose a reason for hiding this comment

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

(duplicated) mark

Jinyu-W
Jinyu-W reviewed May 30, 2022
View reviewed changes
scripts/random_config.py
@@ -139,11 +139,20 @@
"""


def generate_config(sku_num: int, supplier_num: int, warehouse_num: int, retailer_num: int, grid_width: int,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this file should be checked out from this branch

Jinyu-W
Jinyu-W reviewed May 30, 2022
View reviewed changes
tests/cim/test_cim_scenario.py
75,
84,
67,
53,
Copy link
Collaborator

Choose a reason for hiding this comment

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

mark ><

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add exclude mark.

Jinyu-W
Jinyu-W reviewed May 30, 2022
View reviewed changes
maro/cli/utils/node_admin.py
@@ -7,28 +7,22 @@


def start_admin(*args, **kwargs):
print("""If got python module or file not found error, please run
print(
"""If got python module or file not found error, please run
Copy link
Collaborator

Choose a reason for hiding this comment

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

mark

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix the source code.

@Jinyu-W
Copy link
Collaborator

Jinyu-W commented May 30, 2022

Still some isort & flake8 issues.

Seems like it is a conflict?

2022-05-26 03:53:27 [ERROR]   Found errors in [flake8] linter!
2022-05-26 03:53:27 [ERROR]   Error code: 1. Command output:
------
/github/workspace/maro/simulator/scenarios/matrix_accessor.py:80:32: E203 whitespace before ':'

@lihuoran
Copy link
Contributor Author

/github/workspace/maro/simulator/scenarios/matrix_accessor.py:80:32: E203 whitespace before ':'

See this discussion

@Jinyu-W Jinyu-W closed this Jun 13, 2022
@Jinyu-W Jinyu-W deleted the black_format branch June 13, 2022 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jinyu-W Jinyu-W Jinyu-W left review comments

@dongguangbin dongguangbin dongguangbin left review comments

@v-heli v-heli v-heli left review comments

@VictorYXL VictorYXL Awaiting requested review from VictorYXL

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lihuoran @Jinyu-W @dongguangbin @v-heli