Merge pull request #5078 from CoinAlpha/fix/trade_fill_CSV_export_col…

…umns

fix/trade fill CSV export columns

hummingbot/connector/markets_recorder.py

@@ -298,14 +298,6 @@ def _did_complete_funding_payment(self,
                                                                             amount=float(evt.amount))
                     session.add(funding_payment_record)
 
-    @staticmethod
-    def _is_primitive_type(obj: object) -> bool:
-        return not hasattr(obj, '__dict__')
-
-    @staticmethod
-    def _is_protected_method(method_name: str) -> bool:
-        return method_name.startswith('_')
-
     @staticmethod
     def _csv_matches_header(file_path: str, header: tuple) -> bool:
         df = pd.read_csv(file_path, header=None)
@@ -315,10 +307,7 @@ def append_to_csv(self, trade: TradeFill):
         csv_filename = "trades_" + trade.config_file_path[:-4] + ".csv"
         csv_path = os.path.join(data_path(), csv_filename)
 
-        field_names = ("exchange_trade_id",)  # id field should be first
-        field_names += tuple(attr for attr in dir(trade) if (not self._is_protected_method(attr) and
-                                                             self._is_primitive_type(getattr(trade, attr)) and
-                                                             (attr not in field_names)))
+        field_names = tuple(trade.attribute_names_for_file_export())
         field_data = tuple(getattr(trade, attr) for attr in field_names)
 
         # adding extra field "age"

hummingbot/model/trade_fill.py

@@ -59,10 +59,10 @@ class TradeFill(HummingbotBase):
 
     def __repr__(self) -> str:
         return f"TradeFill(config_file_path='{self.config_file_path}', strategy='{self.strategy}', " \
-            f"market='{self.market}', symbol='{self.symbol}', base_asset='{self.base_asset}', " \
-            f"quote_asset='{self.quote_asset}', timestamp={self.timestamp}, order_id='{self.order_id}', " \
-            f"trade_type='{self.trade_type}', order_type='{self.order_type}', price={self.price}, amount={self.amount}, " \
-            f"leverage={self.leverage}, trade_fee={self.trade_fee}, exchange_trade_id={self.exchange_trade_id}, position={self.position})"
+               f"market='{self.market}', symbol='{self.symbol}', base_asset='{self.base_asset}', " \
+               f"quote_asset='{self.quote_asset}', timestamp={self.timestamp}, order_id='{self.order_id}', " \
+               f"trade_type='{self.trade_type}', order_type='{self.order_type}', price={self.price}, amount={self.amount}, " \
+               f"leverage={self.leverage}, trade_fee={self.trade_fee}, exchange_trade_id={self.exchange_trade_id}, position={self.position})"
 
     @staticmethod
     def get_trades(sql_session: Session,
@@ -122,7 +122,8 @@ def to_pandas(cls, trades: List):
             # // indicates order is a paper order so 'n/a'. For real orders, calculate age.
             age = "n/a"
             if "//" not in trade.order_id:
-                age = pd.Timestamp(int(trade.timestamp / 1e3 - int(trade.order_id[-16:]) / 1e6), unit='s').strftime('%H:%M:%S')
+                age = pd.Timestamp(int(trade.timestamp / 1e3 - int(trade.order_id[-16:]) / 1e6), unit='s').strftime(
+                    '%H:%M:%S')
             data.append([
                 trade.exchange_trade_id,
                 datetime.fromtimestamp(int(trade.timestamp / 1e3)).strftime("%Y-%m-%d %H:%M:%S"),
@@ -157,3 +158,24 @@ def to_bounty_api_json(trade_fill: "TradeFill") -> Dict[str, Any]:
                 "trade_fee": trade_fill.trade_fee,
             }
         }
+
+    @staticmethod
+    def attribute_names_for_file_export():
+
+        return [
+            "exchange_trade_id",  # Keep the key attribute first in the list
+            "config_file_path",
+            "strategy",
+            "market",
+            "symbol",
+            "base_asset",
+            "quote_asset",
+            "timestamp",
+            "order_id",
+            "trade_type",
+            "order_type",
+            "price",
+            "amount",
+            "leverage",
+            "trade_fee",
+            "position", ]

test/hummingbot/model/test_trade_fill.py

@@ -0,0 +1,87 @@
+import time
+from decimal import Decimal
+from unittest import TestCase
+
+from hummingbot.core.data_type.trade_fee import AddedToCostTradeFee
+from hummingbot.core.event.events import (
+    OrderType,
+    TradeType,
+)
+from hummingbot.model.trade_fill import TradeFill
+
+
+class TradeFillTests(TestCase):
+
+    def setUp(self) -> None:
+        super().setUp()
+        self.display_name = "test_market"
+        self.config_file_path = "test_config"
+        self.strategy_name = "test_strategy"
+
+        self.symbol = "COINALPHAHBOT"
+        self.base = "COINALPHA"
+        self.quote = "HBOT"
+        self.trading_pair = f"{self.base}-{self.quote}"
+
+    def test_attribute_names_for_file_export(self):
+        expected_attributes = [
+            "exchange_trade_id",
+            "config_file_path",
+            "strategy",
+            "market",
+            "symbol",
+            "base_asset",
+            "quote_asset",
+            "timestamp",
+            "order_id",
+            "trade_type",
+            "order_type",
+            "price",
+            "amount",
+            "leverage",
+            "trade_fee",
+            "position", ]
+
+        self.assertEqual(expected_attributes, TradeFill.attribute_names_for_file_export())
+
+    def test_attribute_names_for_file_export_are_valid(self):
+        trade_fill = TradeFill(
+            config_file_path=self.config_file_path,
+            strategy=self.strategy_name,
+            market=self.display_name,
+            symbol=self.symbol,
+            base_asset=self.base,
+            quote_asset=self.quote,
+            timestamp=int(time.time()),
+            order_id="OID1",
+            trade_type=TradeType.BUY.name,
+            order_type=OrderType.LIMIT.name,
+            price=Decimal(1000),
+            amount=Decimal(1),
+            leverage=1,
+            trade_fee=AddedToCostTradeFee().to_json(),
+            exchange_trade_id="EOID1",
+            position="NILL")
+
+        values = [getattr(trade_fill, attribute) for attribute in TradeFill.attribute_names_for_file_export()]
+
+        expected_values = [
+            trade_fill.exchange_trade_id,
+            trade_fill.config_file_path,
+            trade_fill.strategy,
+            trade_fill.market,
+            trade_fill.symbol,
+            trade_fill.base_asset,
+            trade_fill.quote_asset,
+            trade_fill.timestamp,
+            trade_fill.order_id,
+            trade_fill.trade_type,
+            trade_fill.order_type,
+            trade_fill.price,
+            trade_fill.amount,
+            trade_fill.leverage,
+            trade_fill.trade_fee,
+            trade_fill.position,
+        ]
+
+        self.assertEqual(expected_values, values)