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

Plexon2: Fix conversion between C++ tm structure and python datetime #1372

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

zm711
Copy link
Contributor

@zm711 zm711 commented Jan 23, 2024

Fixes #1371.

As explained in the issue the structures are indexed different so we need to +1 to correctly index. Add comment to explain this with links.

@zm711
Copy link
Contributor Author

zm711 commented Jan 23, 2024

@h-mayorquin, I know you read over and help with the dll for plexon2. Do you want to double check this fix?

@alejoe91 alejoe91 added this to the 0.13.0 milestone Jan 24, 2024
@alejoe91 alejoe91 changed the title Fix conversion between C++ tm structure and python datetime Plexon2: Fix conversion between C++ tm structure and python datetime Jan 26, 2024
@apdavison apdavison merged commit 415845e into NeuralEnsemble:master Jan 26, 2024
1 check passed
Copy link
Contributor

@h-mayorquin h-mayorquin left a comment

Choose a reason for hiding this comment

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

I think that being off by a second with the leap second is fine. Glad that @alejoe91 had the time to check it.

@zm711
Copy link
Contributor Author

zm711 commented Jan 26, 2024

@h-mayorquin, my point for leap second is that based on my reading Python datetime will not accept the leap second value

tm can return 60 or 61 to count as leap seconds, but datatime second only accepts 0..59. So in the case that someone records on a leap second then this IO could fail to annotate the date.

@zm711 zm711 deleted the plexon2 branch January 26, 2024 16:06
@h-mayorquin
Copy link
Contributor

What I mean is to just substract 1 and make it 60 in that case. The other solution would be to propagate all the way up to year in th worse case scenario which is indded difficult as I think you were pointing out.

But also, having the current code just failn and then have someone coming here to tell us so we can device a better solutio nwith an example in our hands seems like a good option : )

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