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

functions returns a list of absolute timestamps instead of returning … #271

Closed
wants to merge 3 commits into from

Conversation

gokulbnr
Copy link
Contributor

In my previous commit, absolute system time at the beginning of the aedat (from its header) was being returned separately. Have fused that with the list of timestamps now. This ensures that the returned timestamps list (as a part of the structures array) is the absolute system time. This may help make the data more easy to synchronize with other data sequences.

…the relative timestamps with a seperate start_time.
@biphasic
Copy link
Member

hey @gokulbnr , my preference normally is to provide the raw data as is, to leave it up to the user to modify it if they want to. There might be a certain start timestamp that a user wants to synchronise to, and subtracting that before returning it will make that impossible. One other option that I can think of is an extra parameter

@biphasic
Copy link
Member

also, I fixed the test in the develop branch

@gokulbnr
Copy link
Contributor Author

Hi @biphasic, thanks for taking a look! Yes, it does make sense to let the user decide how to utilize the raw data. Thanks for keeping me in the loop on this!

@biphasic
Copy link
Member

would you like to change this or should I close it?

@gokulbnr
Copy link
Contributor Author

The code before the above commits aligns with the philosophy of returning the relative and absolute timestamps.
I have a few other changes coming up, but they are not related to the timestamps. So this request can be closed.

@biphasic biphasic closed this Dec 13, 2023
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