Improve implementation, validations and testing for Rust instruments #2733
Conversation
nicolad
force-pushed
the
2630
branch
2 times, most recently
from
c4dff38 to
8012dd1
Compare
There was a problem hiding this comment.
Thanks for all the work including the tests @nicolad 🙏
I'll follow up on the comments after merge to establish some more patterns.
We can address the info field specifically on another pass.
|
|
||
| /// # Errors | ||
| /// | ||
| /// Returns `anyhow::Error` if the value is not finite or cannot be converted to a `Price`. |
There was a problem hiding this comment.
We don't need to qualify the error type in the description here. "Returns an error if ..." is enough.
| @@ -556,7 +636,7 @@ pub fn betting() -> BettingInstrument { | |||
| ); | |||
| let selection_id = 50214; | |||
| let selection_name = Ustr::from("Kansas City Chiefs"); | |||
| let selection_handicap = 0.0; // As per betting convention, no handicap | |||
| let selection_handicap = 0.0; | |||
There was a problem hiding this comment.
This comment is still reasonably informative.
| @@ -501,7 +581,7 @@ pub fn option_spread() -> OptionSpread { | |||
| Symbol::from("UD:U$: GN 2534559"), | |||
| AssetClass::FX, | |||
| Some(Ustr::from("XCME")), | |||
| Ustr::from("SR3"), // British Pound futures (option on futures) | |||
| Ustr::from("SR3"), | |||
There was a problem hiding this comment.
This comment is still reasonably informative.
| @@ -464,7 +544,7 @@ pub fn option_contract_appl() -> OptionContract { | |||
| InstrumentId::from("AAPL211217C00150000.OPRA"), | |||
| Symbol::from("AAPL211217C00150000"), | |||
| AssetClass::Equity, | |||
| Some(Ustr::from("GMNI")), // Nasdaq GEMX | |||
| Some(Ustr::from("GMNI")), | |||
There was a problem hiding this comment.
This comment is still reasonably informative.
| #[case(2.345_5, true)] | ||
| #[case(0.000_999_999, false)] | ||
| #[case(0.000_999_999, true)] | ||
| fn quantity_rounding_grid( |
| let step = 10f64.powi(-(price_precision as i32)); | ||
| Price::new(step, price_precision) | ||
| } | ||
|
|
||
| macro_rules! check_positive { |
| ensure!(price_precision >= 0, "price_precision negative"); | ||
| ensure!(size_precision >= 0, "size_precision negative"); | ||
| check_predicate_true(price_precision >= 0, "price_precision negative")?; | ||
| check_predicate_true(size_precision >= 0, "size_precision negative")?; |
There was a problem hiding this comment.
We have more specialized correctness functions which could be used for many of these, e.g.
https://github.com/nautechsystems/nautilus_trader/blob/develop/crates/model/src/data/trade.rs#L76
check_positive_quantity(size, stringify!(size))?;
Instrument→ Rust (make_price,make_qty, tick helpers, notional math, base-qty math)try_make_price/try_make_qty/try_calculate_base_quantityreturnanyhow::Result, while legacy panicking versions call theirtry_cousins.infometadata in PyO3HashMap<String, PyObject>plus getter/setter.to_dict/from_dictfrom RustFixed,Crypto0_01, …)enum_dispatch, case-insensitiveFromStr.price < 0; tests added.rust_decimal::round_dp_with_strategyfor clarityamount,currency, …)validate_instrument_common)ensure!+ helper macrocheck_positive!; covers paired-field, precision and bound checks.rstestcasesproptest) for price/qty factoriesinfoexposure.cargo clippy -D warnings, multi-OS)try_*fallible variants