Skip to content
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

Feature/derived curves #67

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Conversation

PeterRimmer71
Copy link
Contributor

Enable derived time series in Artesian.SDK

@PeterRimmer71 PeterRimmer71 requested review from a team as code owners September 10, 2024 07:54
Artesian/Artesian.SDK/Artesian.SDK.csproj Outdated Show resolved Hide resolved
/// </summary>
/// <param name="lastN">The number of previous versions to extract</param>
/// <returns>DerivedQuery</returns>
public DerivedQuery ForLastNVersions(int lastN)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Derived has Version semantic?
@arkcecchi

Copy link
Contributor

@AndreaCuneo AndreaCuneo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced about this representation. Afair Derived are currently queried via actuals endpoint as Derived are Actuals.

Since API doesn't distinguish it, why should the SDK?

Is perfectly valid to query together an Actual and a Derived so why force the use to use .For derived()?

Similar for Create(). You create an Actual mkdata Derived from others via a config.

Otherwise I think we should introduce a set of endpoints on API for Derived and validate that only Derived IDs are requested.

'till now, for each MKD type, you have a set of dedicated endpoints. Derived break this.

@arkcecchi please review the design decisions as I'm not convinced anymore Derived should be a MarketDataType. We may have in the future Derived Assessments too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants