-
Notifications
You must be signed in to change notification settings - Fork 25
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.
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
feat: add unfinished_candle method #14
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer simply adding a unfinished_candle
getter method which provides even more flexibility compared to simply closing an Aggregator
.
Its a good feature proposal overall, enhancing the flexibility.
We should be careful to ensure that its clear to a library user that accessing a Candle
through a method like this is not recommended as it does not respect the AggregationRule
. But for more advanced use cases this is useful
src/aggregator.rs
Outdated
|
||
/// Close current active candle and return it | ||
/// | ||
/// # Returns: | ||
/// Last active candle, or None if candle was already finalized | ||
fn close(&mut self) -> Option<Candle>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Close current active candle and return it | |
/// | |
/// # Returns: | |
/// Last active candle, or None if candle was already finalized | |
fn close(&mut self) -> Option<Candle>; | |
/// Get a reference to an unfinished `Candle`. | |
/// Accessing a `Candle` using this method does not guarantee that the `AggregationRule` is respected. | |
/// It is generally advised to call `update` instead and use the resulting `Candle` if its `Some`. | |
fn unfinished_candle(&self) -> &Candle; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, that's what I ended up doing - implemented my own aggregator with method like this. Will update the PR
a1d552f
to
b8aac61
Compare
@MathisWellmann updated :) |
Another problem I have faced, is that there is no way to get the latest candle from the aggregator.
Input data:
TimeRule
andAlignedTimeRule
will trigger on a first trade, and will return the candle, but there is no way to get the second candle.I propose to add a new state for candle -
finalized
, and methodclose
to aggregator. This way, user can easily retrieve the latest candle from the aggregator.Let me know if there are other ways to achieve this.