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

Accessors to retrieve first and last data points from TimeWeightSummary #512

Closed
jerryxwu opened this issue Aug 30, 2022 · 3 comments · Fixed by #513
Closed

Accessors to retrieve first and last data points from TimeWeightSummary #512

jerryxwu opened this issue Aug 30, 2022 · 3 comments · Fixed by #513
Assignees

Comments

@jerryxwu
Copy link
Contributor

Creating this issue for tracebility.

A user on Stackoverflow asked to extract the first and the last point value and timestamp from a time_weight object. We think it make sense to add the same accessors to counter_agg as well. There's very little risk of exposing the internal structure of the objects as the first and the last data points are needed for the rollup functions.

Specifically, the following accessors are discussed:

  • first_val / last_val
  • first_time / last_time (perhaps consider naming them as first_at / last_at to be consistent with open_at, close_at, etc.
@rtwalker
Copy link
Contributor

first_time / last_time (perhaps consider naming them as first_at / last_at to be consistent with open_at, close_at, etc.

This is a good point. Should we likewise consider (first|last) instead of (first|last)_val? Or are they too semantically different from the already existing first and last hyperfunctions?

@jerryxwu
Copy link
Contributor Author

Should we likewise consider (first|last) instead of (first|last)_val? Or are they too semantically different from the already existing first and last hyperfunctions?

Good point. I think it makes sense.

@rtwalker
Copy link
Contributor

rtwalker commented Sep 6, 2022

Consensus from standup today:

  • use first_val/last_val and first_time / last_time names
  • change open_at and friends to open_time

@bors bors bot closed this as completed in cf85c41 Sep 6, 2022
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 a pull request may close this issue.

2 participants