Support more Trade properties

[cemlyn007]

Hello!

Really liking your Rust API! Was just wondering if you might entertain the idea of supporting more properties for the Trade type? In particular I would like to the trade exchange to be supported. I am more than happy to work on an MR 😀

Looking forward to hearing from you,

Kind regards,

Cemlyn

[d-e-s-o]

Are you referring to the realtime streaming data?

If so, I believe we can think about making Data generic over the Bar, Quote, and Trade types to use. This way users can overwrite the defaults if they are insufficient for their purposes and deserialize more fields.

I.e., something along the lines of 9552a6f

[cemlyn007]

Hi @d-e-s-o, sounds like a good solution! Before I took your changes, I forked off main and ran the tests where 6 failed:

1. api::v2::account_activities::tests::page_activities:
   0 == 1 src/api/v2/account_activities.rs:624:5
2. api::v2::account_activities::tests::retrieve_after:
   0 == 1 src/api/v2/account_activities.rs:659:5
3. api::v2::account_activities::tests::retrieve_until:
   1 == 2 src/api/v2/account_activities.rs:675:5
4. api::v2::order::tests::submit_unsatisfiable_fractional_order:
   Received unexpected error:
   Endpoint(InvalidInput(Ok(ApiError { code: 40010001, message: "qty must be integer" })))
5. api::v2::order::tests::submit_unsatisfiable_notional_order:
   Received unexpected error:
   Endpoint(InvalidInput(Ok(ApiError { code: 40010001, message: "qty is required" })))
6. api::v2::updates::tests::stream_order_events:
   404 (maybe because I have APCA_API_STREAM_URL=wss://stream.data.sandbox.alpaca.markets?)

Maybe I am doing something wrong for 6 with my environment variables but 4 and 5 sound like it could be something not on my end? Are some tests expected to be run only during market trading hours?

[d-e-s-o]

Interesting. All tests are expected to pass during all hours, but it is hard to actually test that and so it's possible Alpaca reports something weird and undocumented some of the time and stuff breaks. That being said, doesn't look to be the case here, I would say.

• api::v2::account_activities::tests::page_activities:
  0 == 1 src/api/v2/account_activities.rs:624:5
• api::v2::account_activities::tests::retrieve_after:
  0 == 1 src/api/v2/account_activities.rs:659:5
• api::v2::account_activities::tests::retrieve_until:
  1 == 2 src/api/v2/account_activities.rs:675:5

Not sure about those. They have been stable from what I recall. I ran them a few times and they worked fine. You will need to make sure that your account has activities, but judging from what assertions were hit that should be the case.

4. api::v2::order::tests::submit_unsatisfiable_fractional_order:
Received unexpected error:
Endpoint(InvalidInput(Ok(ApiError { code: 40010001, message: "qty must be integer" })))

Is this an older account, created before fractional orders were a thing? You may have to reset it. They had problems in the past where despite support being enabled via the UI, some internal state was screwed up and all sorts of weird things were happening with their API.

5. api::v2::order::tests::submit_unsatisfiable_notional_order:
Received unexpected error:
Endpoint(InvalidInput(Ok(ApiError { code: 40010001, message: "qty is required" })))

Could be the same issue.

6. api::v2::updates::tests::stream_order_events:
404 (maybe because I have APCA_API_STREAM_URL=wss://stream.data.sandbox.alpaca.markets?)

Sounds like a likely culprit.

[cemlyn007]

Thanks @d-e-s-o, I'll reset my account, it's definitely old. As for APCA_API_STREAM_URL I assumed to test with a paper account and took the URL from this section. I'll check these out later and let you know if I make any progress! Thank you and have a good day!

[d-e-s-o]

As for APCA_API_STREAM_URL I assumed to test with a paper account and took the URL from this section. I'll check these out later and let you know if I make any progress!

Not sure when they changed URLs, but the default is already a paper trading API: wss://paper-api.alpaca.markets/stream and it works (you can easily see that when running RUST_LOG=trace cargo test -- stream_order_events --nocapture) (admittedly haven't read through the page much)

[cemlyn007]

Okay my tests all pass after I reset my account, removed the environment variable setting to the URL specified in Alpaca's documentation and bought some stock on my paper account. Now I can have a play with your code!

[cemlyn007]

Tested your commit here using the stream realtime data example. Works like a charm!

Would you like me to contribute some tests like your existing serialize_deserialize_bar, serialize_deserialize_quote and serialize_deserialize_trade with some a custom Bar, Quote and Trade?

[d-e-s-o]

Tested your commit here using the stream realtime data example. Works like a charm!

Thanks for testing!

Would you like me to contribute some tests like your existing serialize_deserialize_bar, serialize_deserialize_quote and serialize_deserialize_trade with some a custom Bar, Quote and Trade?

That would be great!

[cemlyn007]

Good evening!
I did this:

/// A trade for an equity.
#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
struct DetailedTrade
{
  /// The trade's symbol.
  #[serde(rename = "S")]
  symbol: String,
  /// The trade's ID.
  #[serde(rename = "i")]
  trade_id: u64,
  /// The trade's price.
  #[serde(rename = "p")]
  trade_price: Num,
  /// The trade's size.
  #[serde(rename = "s")]
  trade_size: u64,
  /// The trade's conditions.
  #[serde(rename = "c")]
  conditions: Vec<String>,
  /// The trade's time stamp.
  #[serde(rename = "t")]
  timestamp: DateTime<Utc>,
  /// The trade's exchange.
  #[serde(rename = "x")]
  exchange: String,
  /// The trade's tape.
  #[serde(rename = "z")]
  tape: String,
  /// The trade's update. “canceled”, “corrected”, “incorrect”
  #[serde(rename = "u", default)]
  update: Option<String>,
}

/// Check that we can serialize and deserialize the
/// [`DataMessage::Trade`] variant with a DetailedTrade.
#[test]
fn serialize_deserialize_detailed_trade() {
  let json: &str = r#"{
"T": "t",
"i": 96921,
"S": "AAPL",
"x": "D",
"p": 126.55,
"s": 1,
"t": "2021-02-22T15:51:44.208Z",
"c": ["@", "I"],
"z": "C",
"u": "corrected"
}"#;

  let message = json_from_str::<DataMessage<Bar, Quote, DetailedTrade>>(json).unwrap();
  let trade = match &message {
    DataMessage::Trade(trade) => trade,
    _ => panic!("Decoded unexpected message variant: {message:?}"),
  };
  assert_eq!(trade.symbol, "AAPL");
  assert_eq!(trade.trade_id, 96921);
  assert_eq!(trade.trade_price, Num::new(12655, 100));
  assert_eq!(trade.trade_size, 1);

  assert_eq!(
    trade.timestamp,
    DateTime::<Utc>::from_str("2021-02-22T15:51:44.208Z").unwrap()
  );

  assert_eq!(trade.conditions, vec!["@", "I"]);
  assert_eq!(trade.tape, "C");
  assert_eq!(trade.update, Some("corrected".to_string()));

  assert_eq!(
    json_from_str::<DataMessage<Bar, Quote, DetailedTrade>>(&to_json(&message).unwrap()).unwrap(),
    message
  );
}

I have done something similar for the quote. Have three questions:

1. Is this what you expected? (I am a Rust noob)
2. Do you want the DetailedTrade and DetailedQuote to be public?
3. Your current Bar type is already complete so do you want an incomplete bar type for testing?

[d-e-s-o]

1. Is this what you expected? (I am a Rust noob)

Yes, that's fine.

2. Do you want the DetailedTrade and DetailedQuote to be public?

No need to make it public. It's just for testing at this point.

3. Your current Bar type is already complete so do you want an incomplete bar type for testing?

Oh okay. Nah, it's probably fine then. Will leave it configurable anyway, though.

[d-e-s-o]

Completed as per #53