Merge pull request #193 from ALLAN-DIP/add-flake8

Add flake8 to precommit checks

.pre-commit-config.yaml

@@ -44,17 +44,24 @@ repos:
     hooks:
       - id: prettier
         # Older LTS version needed to run on CentOS 7 (e.g., Frontera)
-        language_version: 16.20.0
+        language_version: 16.20.1
         # Workaround from https://github.com/pre-commit/mirrors-prettier/issues/29#issuecomment-1332667344
         additional_dependencies:
           - prettier@2.8.8
   - repo: https://github.com/codespell-project/codespell
-    rev: v2.2.4
+    rev: v2.2.5
     hooks:
       - id: codespell
         additional_dependencies:
           - tomli==2.0.1
   - repo: https://github.com/shellcheck-py/shellcheck-py # Unofficial but maintained by asottile
-    rev: v0.9.0.2
+    rev: v0.9.0.5
     hooks:
       - id: shellcheck
+  - repo: https://github.com/pycqa/flake8
+    # Newer versions do not support Python 3.7
+    rev: 5.0.4
+    hooks:
+      - id: flake8
+        additional_dependencies: ["flake8-bugbear==23.3.12"]
+        args: ["--max-line-length=150", "--ignore=E203,W503", "--per-file-ignores=__init__.py:F401"]

containers/allan_dip_bot/run_bot.py

@@ -4,13 +4,9 @@
 import argparse
 import asyncio
 import random
-import sys
 import time
 from typing import Optional, Type
 
-sys.path.append("..")  # Adds higher directory to python modules path.
-
-
 from diplomacy import connect
 from diplomacy.client.network_game import NetworkGame
 from diplomacy_research.utils.cluster import is_port_opened

src/baseline_bots/__init__.py

@@ -1 +0,0 @@
-from baseline_bots.bots import *

src/baseline_bots/bots/smart_order_accepter_bot.py

@@ -353,7 +353,9 @@ async def respond_to_peace_messages(self, messages_data: MessagesData) -> None:
             # if the stance value is lower than accept_peace_threshold (-2.5 by default)
             # we will reject the peace proposal
             if power_stance[sender] <= self.accept_peace_threshold:
-                await self.send_message(sender, str(REJ(PRP(message))), messages_data)
+                await self.send_message(
+                    sender, str(REJ(PRP(parsed_message))), messages_data
+                )
                 await self.send_intent_log(
                     "I reject the peace proposal from {} because my stance to {} is no greater than {}".format(
                         sender, sender, self.accept_peace_threshold
@@ -708,7 +710,9 @@ async def replace_aggressive_order_to_allies(self) -> None:
                     order, daide_orders, self.allies
                 )
                 await self.send_intent_log(
-                    f"Replacing order {daide_to_dipnet_parsing(str(order))[0]!r} with {daide_to_dipnet_parsing(str(new_order))[0]!r} because we should not be aggressive to allies."
+                    f"Replacing order {daide_to_dipnet_parsing(str(order))[0]!r} "
+                    f"with {daide_to_dipnet_parsing(str(new_order))[0]!r} because "
+                    "we should not be aggressive to allies."
                 )
             else:
                 new_order = order
@@ -765,7 +769,6 @@ async def do_messaging_round(
         valid_proposal_orders = parsed_messages_dict["valid_proposals"]
         invalid_proposal_orders = parsed_messages_dict["invalid_proposals"]
         shared_orders = parsed_messages_dict["shared_orders"]
-        other_orders = parsed_messages_dict["other_orders"]
         self.alliances_prps = parsed_messages_dict["alliance_proposals"]
         self.peace_prps = parsed_messages_dict["peace_proposals"]
 

src/baseline_bots/randomize_order.py

@@ -5,7 +5,7 @@
 
 
 import random
-from typing import List, Tuple, Union
+from typing import List, Tuple, Union  # noqa: F401
 
 from daidepp import (
     BLD,
@@ -152,47 +152,12 @@ def random_list_orders(orders: List[Command]) -> List[Command]:
     :return: The list of deviant orders
     :rtype: List[Tuple]
     """
-    # correspondences = orders_correspondence(
-    #     orders
-    # )  # this returns a list of tuples representing correspondences or an empty list
-
     orders = list(
         map(lambda order: randomize(order), orders)
     )  # if there are no correspondences, every order is randomized alone
     return orders
 
 
-def orders_correspondence(orders: List) -> List:
-    """
-    Checks if there are corresponding orders in the list of orders it
-    takes in. Corresponding orders are orders such as (x SUP y MTO LIV)
-    and (y MTO LIV). The same principle applies to supported holds and
-    convoys.
-
-    :param orders: A list of DAIDE orders in the following format. [((FRA AMY PIC) MTO BRE), ((FRA AMY PIC) HLD), ((FRA AMY BUR) HLD)]
-    :type orders: List[Tuple]
-    :return: The list of all sets (as tuples) of corresponding orders.
-    :rtype: List[Tuple]
-    """
-    correspondences = []  # a list of tuples representing all correspondences
-    for i, order in enumerate(orders):
-        if order[1] == "SUP":
-            if len(order) > 3:  # if it is supporting a move
-                u1, _, u2, _, province = order
-                if (u2, "MTO", province) in orders:
-                    correspondences.append((order, (u2, "MTO", province)))
-            else:  # if it is supporting a hold
-                u1, _, u2 = order
-                if (u2, "HLD") in orders:
-                    correspondences.append((order, (u2, "HLD")))
-        elif order[1] == "CVY":
-            convoy_moves = filter(  # filter all the convoy related moves
-                lambda order: order[1] == "CTO" or order[1] == "CVY", orders
-            )
-            correspondences.append(tuple(convoy_moves))
-    return correspondences
-
-
 def randomize(order: Command) -> Command:
     """
     Takes in an order and returns a randomly deviant version of it.
@@ -263,16 +228,21 @@ def random_convoy(order: CVY) -> CVY:
     :return: A deviant order (with some chance of being the same order).
     :rtype: Tuple
     """
-    # fmt: off
     # TODO: Add to `daidepp`?
-    assert (order.convoyed_unit.unit_type == "AMY" and order.convoying_unit.unit_type == "FLT"), "The unit type is neither army nor fleet so it is invalid."
+    assert (
+        order.convoyed_unit.unit_type == "AMY"
+        and order.convoying_unit.unit_type == "FLT"
+    ), "The unit type is neither army nor fleet so it is invalid."
     # It is necessary to check whether a possible alternate "convoy-to" location is adjacent to the unit being convoyed
     # since convoying to a province adjacent to you would be less believable
     flt_loc = dipnetify_location(order.convoying_unit.location)
     amy_loc = dipnetify_location(order.convoyed_unit.location)
     province = dipnetify_location(order.province)
-    adj = [str(daidefy_location(loc)) for loc in ADJACENCY[flt_loc] if TYPES[loc] == "COAST" and loc not in ADJACENCY[amy_loc] and loc != province]
-    # fmt: on
+    adj = [
+        str(daidefy_location(loc))
+        for loc in ADJACENCY[flt_loc]
+        if TYPES[loc] == "COAST" and loc not in ADJACENCY[amy_loc] and loc != province
+    ]
     if adj:  # if valid adjacencies exist
         return CVY(
             order.convoying_unit,
@@ -321,7 +291,6 @@ def random_support(order: SUP) -> SUP:
             # returns the same support hold order if there is no value adjacent to both
             return order
     else:  # if it is supporting to move
-        # fmt: off
         sup_type = order.supporting_unit.unit_type
         sup_loc = dipnetify_location(order.supporting_unit.location)
         rec_type = order.supported_unit.unit_type
@@ -330,8 +299,11 @@ def random_support(order: SUP) -> SUP:
         sup_adjacent, rec_adjacent = ADJACENCY[sup_loc], ADJACENCY[rec_loc]
         # COMBOS and TYPES must be used to determine the possible locations a unit can support into/from based on the unit type and province type
         dest_choices = COMBOS[sup_type][rec_type]
-        adj_to_both = [daidefy_location(adjacency).province for adjacency in sup_adjacent if adjacency in rec_adjacent and adjacency != province and TYPES[adjacency]]
-        # fmt: on
+        adj_to_both = [
+            daidefy_location(adjacency).province
+            for adjacency in sup_adjacent
+            if adjacency in rec_adjacent and adjacency != province and TYPES[adjacency]
+        ]
         if adj_to_both:
             return SUP(
                 order.supporting_unit, order.supported_unit, random.choice(adj_to_both)

src/baseline_bots/utils.py

@@ -313,9 +313,6 @@ async def get_state_value(
 ) -> int:
     # rollout the game --- orders in rollout are from dipnet
     # state value
-    firststep_sc = len(game.get_centers(power_name))
-    dipnet_comparison = {power: 0 for power in game.map.powers}
-    support_count = {power: 0 for power in game.map.powers}
     movement_phase = 0
     for i in range(3 * bot.rollout_length):
         if game.get_current_phase().endswith("M"):
@@ -390,11 +387,11 @@ def __deepcopy__(game):
             ]:
                 continue
             setattr(result, key, deepcopy(getattr(game, key)))
-        setattr(result, "map", game.map)
-        setattr(result, "powers", {})
+        result.map = game.map
+        result.powers = {}
         for power in game.powers.values():
             result.powers[power.name] = deepcopy(power)
-            setattr(result.powers[power.name], "game", result)
+            result.powers[power.name].game = result
         result.role = strings.SERVER_TYPE
         return result
 
@@ -405,8 +402,6 @@ def __deepcopy__(game):
     for proposer, unit_orders in proposal_order.items():
         # if there is a proposal from this power
         if unit_orders:
-            proposed = True
-
             # simulate game by copying the current one
             simulated_game = __deepcopy__(bot.game)
 

tests/bot_tests/SOA_test.py

@@ -72,7 +72,7 @@ def test_send_message(self):
                     daide_port=None,
                 )
                 game_created = True
-            except:
+            except Exception:
                 # game not created because of same game id
                 pass
 
@@ -106,7 +106,8 @@ def test_send_message(self):
             msgs, done = yield game_play.step()
             test_rounds_count -= 1
 
-            # Check any other country (randomly chosen RUSSIA here for this purpose) for messages received. SOA bot by design sends ALY message to all other bots
+            # Check any other country (randomly chosen RUSSIA here for this purpose)
+            # for messages received. SOA bot by design sends ALY message to all other bots
             rcvd_messages = list(
                 game_play.game.filter_messages(
                     messages=game_play.game.messages, game_role="RUSSIA"
@@ -116,7 +117,8 @@ def test_send_message(self):
             # message count should be non-zero
             assert len(rcvd_messages) != 0
 
-            # Note this is a valid test case since we know ALY is sent by SOA bot to all other powers in the beginning and this is the only bot amongst 7 powers
+            # Note this is a valid test case since we know ALY is sent by SOA bot to
+            # all other powers in the beginning and this is the only bot amongst 7 powers
             assert any(["ALY" in msg.message for msg in rcvd_messages])
         print("finish test_send_message")
 
@@ -162,17 +164,17 @@ def test_respond_to_alliance_messages(self):
                 [
                     {
                         "recipient": "RUSSIA",
-                        "message": "YES (ALY (TUR RUS ENG ITA) VSS (FRA GER AUS))",
+                        "message": "YES ( PRP ( ALY ( ENG ITA RUS TUR ) VSS ( AUS FRA GER ) ) )",
                     }
                 ],
             ]
         ]
 
         for tc_ip, tc_op in RESPOND_TO_ALLIANCES_TC:
             msg_data = MessagesData()
-            soa_bot.alliances = tc_ip
+            soa_bot.alliances_prps = tc_ip
             yield soa_bot.respond_to_alliance_messages(msg_data)
-            # assert msg_data.messages == tc_op, (msg_data.messages, tc_op)
+            assert msg_data.messages == tc_op, (msg_data.messages, tc_op)
 
         stop_io_loop()
 
@@ -344,7 +346,7 @@ def test_parse_proposals(self):
 
         soa_power_units = game_play.game.powers[soa_bot.power_name].units[:]
 
-        for power, orders in valid_proposal_orders.items():
+        for orders in valid_proposal_orders.values():
             for order in orders:
                 order_token = get_order_tokens(order)
                 unit_order = order_token[0]
@@ -389,7 +391,7 @@ def test_gen_pos_stance_messages(self):
         game_play.game.set_centers("RUSSIA", ["MOS"], reset=True)
         game_play.game.set_centers("GERMANY", ["MUN", "KIE", "BER", "BEL"])
         game_play.game.set_centers(soa_bot.power_name, ["PAR", "BRE", "MAR"])
-        ret_data = yield soa_bot()
+        yield soa_bot()
         soa_bot_stance = soa_bot.stance.get_stance()[soa_bot.power_name]
         print(game_play.game.get_centers())
         print("expected stance ENGLAND: 1, RUSSIA: 1, GERMANY: -1, AUTRIA:0")

tests/utils_test.py

@@ -219,7 +219,8 @@ def test_dipnet_to_daide_parsing_convoys(
         [
             "TURKEY",
             {
-                "RUSSIA": "PRP(AND (XDO((TUR FLT ANK) MTO BLA)) (XDO((RUS AMY SEV) MTO RUM)) (XDO((ENG AMY LVP) HLD)) (ALY (TUR RUS ENG ITA) VSS (FRA GER AUS)) (PCE (TUR RUS) ))"
+                "RUSSIA": "PRP(AND (XDO((TUR FLT ANK) MTO BLA)) (XDO((RUS AMY SEV) MTO RUM)) "
+                "(XDO((ENG AMY LVP) HLD)) (ALY (TUR RUS ENG ITA) VSS (FRA GER AUS)) (PCE (TUR RUS) ))"
             },
             {
                 "valid_proposals": {"RUSSIA": ["F ANK - BLA"]},